Skip to content

Commit 077c31c

Browse files
DrJosh9000DrJosh9000
authored
Merge pull request #3338 from buildkite/ps-732-remove-path-check
Self-execute the path from os.Executable in more places
2 parents 030b328 + 2a313c5 commit 077c31c

15 files changed

Lines changed: 146 additions & 145 deletions

‎clicommand/agent_start.go‎

Lines changed: 0 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"io"
99
"maps"
1010
"os"
11-
"os/exec"
1211
"os/signal"
1312
"path/filepath"
1413
"regexp"
@@ -788,18 +787,6 @@ var AgentStartCommand = cli.Command{
788787
// used later to force the shutdown of the agent
789788
ctx, cancel := context.WithCancel(ctx)
790789

791-
// Verify that the bootstrap buildkite-agent executable matches the current host agent
792-
// executable. In development builds, it exits on mismatch; otherwise it only logs a warning.
793-
// This is to avoid confusion when making changes to the buildkite-agent but forgetting to
794-
// update the buildkite-agent binary available from $PATH
795-
if err := checkBinaryPaths(); err != nil {
796-
if version.IsDevelopmentBuild() {
797-
return fmt.Errorf("check binary paths: %s", err)
798-
} else {
799-
l.Warn("check binary paths: %s", err)
800-
}
801-
}
802-
803790
// Remove any config env from the environment to prevent them propagating to bootstrap
804791
if err := UnsetConfigFromEnvironment(c); err != nil {
805792
return fmt.Errorf("failed to unset config from environment: %w", err)
@@ -1302,39 +1289,6 @@ var AgentStartCommand = cli.Command{
13021289
},
13031290
}
13041291

1305-
// checkBinaryPaths looks up both the bootstrap and host buildkite-agent paths,
1306-
// and returns an error if they do not match or if either cannot be determined.
1307-
func checkBinaryPaths() error {
1308-
bootstrapPath, err := exec.LookPath("buildkite-agent")
1309-
if err != nil {
1310-
return fmt.Errorf("failed to locate bootstrap buildkite-agent: %w", err)
1311-
}
1312-
1313-
evalBootstrapPath, err := filepath.EvalSymlinks(bootstrapPath)
1314-
if err != nil {
1315-
return fmt.Errorf("failed to locate bootstrap buildkite-agent: %w", err)
1316-
}
1317-
1318-
hostPath, err := os.Executable()
1319-
if err != nil {
1320-
return fmt.Errorf("failed to determine host buildkite-agent executable: %w", err)
1321-
}
1322-
1323-
evalHostPath, err := filepath.EvalSymlinks(hostPath)
1324-
if err != nil {
1325-
return fmt.Errorf("failed to determine host buildkite-agent executable: %w", err)
1326-
}
1327-
1328-
if evalHostPath != evalBootstrapPath {
1329-
return fmt.Errorf(
1330-
"mismatched buildkite-agent paths: host=%q bootstrap=%q",
1331-
evalHostPath, evalBootstrapPath,
1332-
)
1333-
}
1334-
1335-
return nil
1336-
}
1337-
13381292
func parseAndValidateJWKS(ctx context.Context, keysetType, path string) (jwk.Set, error) {
13391293
jwksBytes, err := os.ReadFile(path)
13401294
if err != nil {

‎clicommand/bootstrap.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"syscall"
1111

1212
"github.com/buildkite/agent/v3/internal/job"
13+
"github.com/buildkite/agent/v3/internal/self"
1314
"github.com/buildkite/agent/v3/process"
1415
"github.com/buildkite/agent/v3/tracetools"
1516
"github.com/urfave/cli"
@@ -402,6 +403,17 @@ var BootstrapCommand = cli.Command{
402403
ctx, cfg, l, _, done := setupLoggerAndConfig[BootstrapConfig](ctx, c)
403404
defer done()
404405

406+
// Secret env var that overrides the self-execution path, but only if
407+
// the job ID is a magical, nonsensical value (set by the
408+
// ExecutorTester).
409+
// This does not alter what 'buildkite-agent' means within a command;
410+
// for that, we would have to mess with $PATH.
411+
if cfg.JobID == "1111-1111-1111-1111" {
412+
if overrideSelf := os.Getenv("BUILDKITE_OVERRIDE_SELF"); overrideSelf != "" {
413+
ctx = self.OverridePath(ctx, overrideSelf)
414+
}
415+
}
416+
405417
// Turn of PTY support if we're on Windows
406418
runInPty := cfg.PTY
407419
if runtime.GOOS == "windows" {

‎internal/job/artifacts.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package job
33
import (
44
"context"
55

6+
"github.com/buildkite/agent/v3/internal/self"
67
"github.com/buildkite/agent/v3/tracetools"
78
)
89

@@ -69,7 +70,7 @@ func (e *Executor) uploadArtifacts(ctx context.Context) error {
6970
args = append(args, e.ArtifactUploadDestination)
7071
}
7172

72-
if err = e.shell.Command("buildkite-agent", args...).Run(ctx); err != nil {
73+
if err = e.shell.Command(self.Path(ctx), args...).Run(ctx); err != nil {
7374
return err
7475
}
7576

‎internal/job/checkout.go‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212

1313
"github.com/buildkite/agent/v3/internal/experiments"
1414
"github.com/buildkite/agent/v3/internal/osutil"
15+
"github.com/buildkite/agent/v3/internal/self"
1516
"github.com/buildkite/agent/v3/internal/shell"
1617
"github.com/buildkite/agent/v3/tracetools"
1718
"github.com/buildkite/roko"
@@ -32,12 +33,7 @@ func (e *Executor) configureGitCredentialHelper(ctx context.Context) error {
3233
return fmt.Errorf("enabling git credential.useHttpPath: %w", err)
3334
}
3435

35-
buildkiteAgent, err := os.Executable()
36-
if err != nil {
37-
return fmt.Errorf("getting executable path: %w", err)
38-
}
39-
40-
helper := fmt.Sprintf(`%s git-credentials-helper`, buildkiteAgent)
36+
helper := fmt.Sprintf(`%s git-credentials-helper`, self.Path(ctx))
4137
err = e.shell.Command("git", "config", "--global", "credential.helper", helper).Run(ctx, shell.ShowPrompt(false))
4238
if err != nil {
4339
return fmt.Errorf("configuring git credential.helper: %w", err)
@@ -853,7 +849,7 @@ func (e *Executor) sendCommitToBuildkite(ctx context.Context) error {
853849
return nil
854850
}
855851

856-
cmd := e.shell.Command("buildkite-agent", "meta-data", "exists", CommitMetadataKey)
852+
cmd := e.shell.Command(self.Path(ctx), "meta-data", "exists", CommitMetadataKey)
857853
if err := cmd.Run(ctx); err == nil {
858854
// Command exited 0, ie the key exists, so we don't need to send it again
859855
e.shell.Commentf("Git commit information has already been sent to Buildkite")
@@ -886,7 +882,7 @@ func (e *Executor) sendCommitToBuildkite(ctx context.Context) error {
886882
}
887883

888884
stdin := strings.NewReader(out)
889-
cmd = e.shell.CloneWithStdin(stdin).Command("buildkite-agent", "meta-data", "set", CommitMetadataKey)
885+
cmd = e.shell.CloneWithStdin(stdin).Command(self.Path(ctx), "meta-data", "set", CommitMetadataKey)
890886
if err := cmd.Run(ctx); err != nil {
891887
return fmt.Errorf("sending git commit information to Buildkite: %w", err)
892888
}

‎internal/job/integration/artifact_integration_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ func TestArtifactsUploadAfterCommand(t *testing.T) {
1414

1515
tester, err := NewExecutorTester(mainCtx)
1616
if err != nil {
17-
t.Fatalf("NewBootstrapTester() error = %v", err)
17+
t.Fatalf("NewExecutorTester() error = %v", err)
1818
}
1919
defer tester.Close()
2020

@@ -43,7 +43,7 @@ func TestArtifactsUploadAfterCommandFails(t *testing.T) {
4343

4444
tester, err := NewExecutorTester(mainCtx)
4545
if err != nil {
46-
t.Fatalf("NewBootstrapTester() error = %v", err)
46+
t.Fatalf("NewExecutorTester() error = %v", err)
4747
}
4848
defer tester.Close()
4949

@@ -77,7 +77,7 @@ func TestArtifactsUploadAfterCommandHookFails(t *testing.T) {
7777

7878
tester, err := NewExecutorTester(mainCtx)
7979
if err != nil {
80-
t.Fatalf("NewBootstrapTester() error = %v", err)
80+
t.Fatalf("NewExecutorTester() error = %v", err)
8181
}
8282
defer tester.Close()
8383

‎internal/job/integration/checkout_git_mirrors_integration_test.go‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ func TestCheckingOutGitHubPullRequests_WithGitMirrors(t *testing.T) {
2020

2121
tester, err := NewExecutorTester(mainCtx)
2222
if err != nil {
23-
t.Fatalf("NewBootstrapTester() error = %v", err)
23+
t.Fatalf("NewExecutorTester() error = %v", err)
2424
}
2525
defer tester.Close()
2626

@@ -65,7 +65,7 @@ func TestWithResolvingCommitExperiment_WithGitMirrors(t *testing.T) {
6565
ctx, _ := experiments.Enable(mainCtx, experiments.ResolveCommitAfterCheckout)
6666
tester, err := NewExecutorTester(ctx)
6767
if err != nil {
68-
t.Fatalf("NewBootstrapTester() error = %v", err)
68+
t.Fatalf("NewExecutorTester() error = %v", err)
6969
}
7070
defer tester.Close()
7171

@@ -104,7 +104,7 @@ func TestCheckingOutLocalGitProject_WithGitMirrors(t *testing.T) {
104104

105105
tester, err := NewExecutorTester(mainCtx)
106106
if err != nil {
107-
t.Fatalf("NewBootstrapTester() error = %v", err)
107+
t.Fatalf("NewExecutorTester() error = %v", err)
108108
}
109109
defer tester.Close()
110110

@@ -153,7 +153,7 @@ func TestCheckingOutLocalGitProjectWithSubmodules_WithGitMirrors(t *testing.T) {
153153

154154
tester, err := NewExecutorTester(mainCtx)
155155
if err != nil {
156-
t.Fatalf("NewBootstrapTester() error = %v", err)
156+
t.Fatalf("NewExecutorTester() error = %v", err)
157157
}
158158
defer tester.Close()
159159

@@ -225,7 +225,7 @@ func TestCheckingOutLocalGitProjectWithSubmodulesDisabled_WithGitMirrors(t *test
225225

226226
tester, err := NewExecutorTester(mainCtx)
227227
if err != nil {
228-
t.Fatalf("NewBootstrapTester() error = %v", err)
228+
t.Fatalf("NewExecutorTester() error = %v", err)
229229
}
230230
defer tester.Close()
231231

@@ -286,7 +286,7 @@ func TestCheckingOutShallowCloneOfLocalGitProject_WithGitMirrors(t *testing.T) {
286286

287287
tester, err := NewExecutorTester(mainCtx)
288288
if err != nil {
289-
t.Fatalf("NewBootstrapTester() error = %v", err)
289+
t.Fatalf("NewExecutorTester() error = %v", err)
290290
}
291291
defer tester.Close()
292292

@@ -330,7 +330,7 @@ func TestCheckingOutSetsCorrectGitMetadataAndSendsItToBuildkite_WithGitMirrors(t
330330

331331
tester, err := NewExecutorTester(mainCtx)
332332
if err != nil {
333-
t.Fatalf("NewBootstrapTester() error = %v", err)
333+
t.Fatalf("NewExecutorTester() error = %v", err)
334334
}
335335
defer tester.Close()
336336

@@ -350,7 +350,7 @@ func TestCheckingOutWithSSHKeyscan_WithGitMirrors(t *testing.T) {
350350

351351
tester, err := NewExecutorTester(mainCtx)
352352
if err != nil {
353-
t.Fatalf("NewBootstrapTester() error = %v", err)
353+
t.Fatalf("NewExecutorTester() error = %v", err)
354354
}
355355
defer tester.Close()
356356

@@ -382,7 +382,7 @@ func TestCheckingOutWithoutSSHKeyscan_WithGitMirrors(t *testing.T) {
382382

383383
tester, err := NewExecutorTester(mainCtx)
384384
if err != nil {
385-
t.Fatalf("NewBootstrapTester() error = %v", err)
385+
t.Fatalf("NewExecutorTester() error = %v", err)
386386
}
387387
defer tester.Close()
388388

@@ -407,7 +407,7 @@ func TestCheckingOutWithSSHKeyscanAndUnscannableRepo_WithGitMirrors(t *testing.T
407407

408408
tester, err := NewExecutorTester(mainCtx)
409409
if err != nil {
410-
t.Fatalf("NewBootstrapTester() error = %v", err)
410+
t.Fatalf("NewExecutorTester() error = %v", err)
411411
}
412412
defer tester.Close()
413413

@@ -440,7 +440,7 @@ func TestCleaningAnExistingCheckout_WithGitMirrors(t *testing.T) {
440440

441441
tester, err := NewExecutorTester(mainCtx)
442442
if err != nil {
443-
t.Fatalf("NewBootstrapTester() error = %v", err)
443+
t.Fatalf("NewExecutorTester() error = %v", err)
444444
}
445445
defer tester.Close()
446446

@@ -480,7 +480,7 @@ func TestForcingACleanCheckout_WithGitMirrors(t *testing.T) {
480480

481481
tester, err := NewExecutorTester(mainCtx)
482482
if err != nil {
483-
t.Fatalf("NewBootstrapTester() error = %v", err)
483+
t.Fatalf("NewExecutorTester() error = %v", err)
484484
}
485485
defer tester.Close()
486486

@@ -506,7 +506,7 @@ func TestCheckoutOnAnExistingRepositoryWithoutAGitFolder_WithGitMirrors(t *testi
506506

507507
tester, err := NewExecutorTester(mainCtx)
508508
if err != nil {
509-
t.Fatalf("NewBootstrapTester() error = %v", err)
509+
t.Fatalf("NewExecutorTester() error = %v", err)
510510
}
511511
defer tester.Close()
512512

@@ -537,7 +537,7 @@ func TestCheckoutRetriesOnCleanFailure_WithGitMirrors(t *testing.T) {
537537

538538
tester, err := NewExecutorTester(mainCtx)
539539
if err != nil {
540-
t.Fatalf("NewBootstrapTester() error = %v", err)
540+
t.Fatalf("NewExecutorTester() error = %v", err)
541541
}
542542
defer tester.Close()
543543

@@ -569,7 +569,7 @@ func TestCheckoutRetriesOnCloneFailure_WithGitMirrors(t *testing.T) {
569569

570570
tester, err := NewExecutorTester(mainCtx)
571571
if err != nil {
572-
t.Fatalf("NewBootstrapTester() error = %v", err)
572+
t.Fatalf("NewExecutorTester() error = %v", err)
573573
}
574574
defer tester.Close()
575575

@@ -599,7 +599,7 @@ func TestCheckoutDoesNotRetryOnHookFailure_WithGitMirrors(t *testing.T) {
599599

600600
tester, err := NewExecutorTester(mainCtx)
601601
if err != nil {
602-
t.Fatalf("NewBootstrapTester() error = %v", err)
602+
t.Fatalf("NewExecutorTester() error = %v", err)
603603
}
604604
defer tester.Close()
605605

@@ -636,7 +636,7 @@ func TestRepositorylessCheckout_WithGitMirrors(t *testing.T) {
636636

637637
tester, err := NewExecutorTester(mainCtx)
638638
if err != nil {
639-
t.Fatalf("NewBootstrapTester() error = %v", err)
639+
t.Fatalf("NewExecutorTester() error = %v", err)
640640
}
641641
defer tester.Close()
642642

@@ -669,7 +669,7 @@ func TestGitMirrorEnv(t *testing.T) {
669669

670670
tester, err := NewExecutorTester(mainCtx)
671671
if err != nil {
672-
t.Fatalf("NewBootstrapTester() error = %v", err)
672+
t.Fatalf("NewExecutorTester() error = %v", err)
673673
}
674674
defer tester.Close()
675675

0 commit comments

Comments
 (0)