Merge pull request #3124 from buildkite/feature/additional-hooks-paths

Adding support for Additional Hooks Paths

agent/agent_configuration.go

@@ -12,6 +12,7 @@ type AgentConfiguration struct {
 	BootstrapScript             string
 	BuildPath                   string
 	HooksPath                   string
+	AdditionalHooksPaths        []string
 	SocketsPath                 string
 	GitMirrorsPath              string
 	GitMirrorsLockTimeout       int

agent/job_runner.go

@@ -512,6 +512,7 @@ func (r *JobRunner) createEnvironment(ctx context.Context) ([]string, error) {
 	env["BUILDKITE_GIT_MIRRORS_PATH"] = r.conf.AgentConfiguration.GitMirrorsPath
 	env["BUILDKITE_GIT_MIRRORS_SKIP_UPDATE"] = fmt.Sprint(r.conf.AgentConfiguration.GitMirrorsSkipUpdate)
 	env["BUILDKITE_HOOKS_PATH"] = r.conf.AgentConfiguration.HooksPath
+	env["BUILDKITE_ADDITIONAL_HOOKS_PATHS"] = strings.Join(r.conf.AgentConfiguration.AdditionalHooksPaths, ",")
 	env["BUILDKITE_PLUGINS_PATH"] = r.conf.AgentConfiguration.PluginsPath
 	env["BUILDKITE_SSH_KEYSCAN"] = fmt.Sprint(r.conf.AgentConfiguration.SSHKeyscan)
 	env["BUILDKITE_GIT_SUBMODULES"] = fmt.Sprint(r.conf.AgentConfiguration.GitSubmodules)

clicommand/agent_start.go

@@ -110,10 +110,11 @@ type AgentStartConfig struct {
 	WriteJobLogsToStdout bool     `cli:"write-job-logs-to-stdout"`
 	DisableWarningsFor   []string `cli:"disable-warnings-for" normalize:"list"`
 
-	BuildPath   string `cli:"build-path" normalize:"filepath" validate:"required"`
-	HooksPath   string `cli:"hooks-path" normalize:"filepath"`
-	SocketsPath string `cli:"sockets-path" normalize:"filepath"`
-	PluginsPath string `cli:"plugins-path" normalize:"filepath"`
+	BuildPath            string   `cli:"build-path" normalize:"filepath" validate:"required"`
+	HooksPath            string   `cli:"hooks-path" normalize:"filepath"`
+	AdditionalHooksPaths []string `cli:"additional-hooks-paths" normalize:"list"`
+	SocketsPath          string   `cli:"sockets-path" normalize:"filepath"`
+	PluginsPath          string   `cli:"plugins-path" normalize:"filepath"`
 
 	Shell           string `cli:"shell"`
 	BootstrapScript string `cli:"bootstrap-script" normalize:"commandpath"`
@@ -509,6 +510,12 @@ var AgentStartCommand = cli.Command{
 			Usage:  "Directory where the hook scripts are found",
 			EnvVar: "BUILDKITE_HOOKS_PATH",
 		},
+		cli.StringSliceFlag{
+			Name:   "additional-hooks-paths",
+			Value:  &cli.StringSlice{},
+			Usage:  "Additional directories to look for agent hooks",
+			EnvVar: "BUILDKITE_ADDITIONAL_HOOKS_PATHS",
+		},
 		cli.StringFlag{
 			Name:   "sockets-path",
 			Value:  defaultSocketsPath(),
@@ -978,6 +985,7 @@ var AgentStartCommand = cli.Command{
 			GitMirrorsLockTimeout:        cfg.GitMirrorsLockTimeout,
 			GitMirrorsSkipUpdate:         cfg.GitMirrorsSkipUpdate,
 			HooksPath:                    cfg.HooksPath,
+			AdditionalHooksPaths:         cfg.AdditionalHooksPaths,
 			PluginsPath:                  cfg.PluginsPath,
 			GitCheckoutFlags:             cfg.GitCheckoutFlags,
 			GitCloneFlags:                cfg.GitCloneFlags,
@@ -1060,6 +1068,7 @@ var AgentStartCommand = cli.Command{
 		l.Debug("Bootstrap command: %s", agentConf.BootstrapScript)
 		l.Debug("Build path: %s", agentConf.BuildPath)
 		l.Debug("Hooks directory: %s", agentConf.HooksPath)
+		l.Debug("Additional hooks directories: %v", agentConf.AdditionalHooksPaths)
 		l.Debug("Plugins directory: %s", agentConf.PluginsPath)
 
 		if exps := experiments.KnownAndEnabled(ctx); len(exps) > 0 {
@@ -1389,13 +1398,27 @@ func agentShutdownHook(log logger.Logger, cfg AgentStartConfig) {
 // agent logger. Exit status failure is logged and returned for the caller to handle
 func agentLifecycleHook(hookName string, log logger.Logger, cfg AgentStartConfig) error {
 	// search for hook (including .bat & .ps1 files on Windows)
+	hooks := []string{}
 	p, err := hook.Find(cfg.HooksPath, hookName)
 	if err != nil {
 		if !os.IsNotExist(err) {
 			log.Error("Error finding %q hook: %v", hookName, err)
 			return err
 		}
-		return nil
+	} else {
+		hooks = append(hooks, p)
+	}
+
+	// also search for hook in any additionally provided locations
+	for _, h := range cfg.AdditionalHooksPaths {
+		p, err = hook.Find(h, hookName)
+		if err != nil {
+			if !os.IsNotExist(err) {
+				log.Error("Error finding %q hook: %v", hookName, err)
+			}
+		} else {
+			hooks = append(hooks, p)
+		}
 	}
 
 	// pipe from hook output to logger
@@ -1420,22 +1443,24 @@ func agentLifecycleHook(hookName string, log logger.Logger, cfg AgentStartConfig
 		}
 	}()
 
-	// run hook
-	script, err := sh.Script(p)
-	if err != nil {
-		log.Error("%q hook: %v", hookName, err)
-		return err
-	}
-	// For these hooks, hide the interpreter from the "prompt".
-	sh.Promptf("%s", p)
-	if err := script.Run(context.TODO(), shell.ShowPrompt(false)); err != nil {
-		log.Error("%q hook: %v", hookName, err)
-		return err
-	}
-	w.Close() // goroutine scans until pipe is closed
+	// run hooks
+	for _, p = range hooks {
+		script, err := sh.Script(p)
+		if err != nil {
+			log.Error("%q hook: %v", hookName, err)
+			return err
+		}
+		// For these hooks, hide the interpreter from the "prompt".
+		sh.Promptf("%s", p)
+		if err := script.Run(context.TODO(), shell.ShowPrompt(false)); err != nil {
+			log.Error("%q hook: %v", hookName, err)
+			return err
+		}
+		w.Close() // goroutine scans until pipe is closed
 
-	// wait for hook to finish and output to flush to logger
-	wg.Wait()
+		// wait for hook to finish and output to flush to logger
+		wg.Wait()
+	}
 	return nil
 }
 

clicommand/agent_start_test.go

@@ -20,21 +20,23 @@ func setupHooksPath(t *testing.T) (string, func()) {
 	return hooksPath, func() { os.RemoveAll(hooksPath) }
 }
 
-func writeAgentHook(t *testing.T, dir, hookName string) string {
+func writeAgentHook(t *testing.T, dir, hookName, msg string) string {
 	t.Helper()
 
 	var filename, script string
 	if runtime.GOOS == "windows" {
 		filename = hookName + ".bat"
-		script = "@echo off\necho hello world"
+		script = "@echo off\necho " + msg
 	} else {
 		filename = hookName
-		script = "echo hello world"
+		script = "echo " + msg
 	}
 	filepath := filepath.Join(dir, filename)
+	t.Logf("Creating %q with %q content", filepath, msg)
 	if err := os.WriteFile(filepath, []byte(script), 0o755); err != nil {
 		assert.FailNow(t, "failed to write %q hook: %v", hookName, err)
 	}
+	t.Log("Providing the path with file created")
 	return filepath
 }
 
@@ -57,7 +59,7 @@ func TestAgentStartupHook(t *testing.T) {
 
 		hooksPath, closer := setupHooksPath(t)
 		defer closer()
-		filepath := writeAgentHook(t, hooksPath, "agent-startup")
+		filepath := writeAgentHook(t, hooksPath, "agent-startup", "hello world")
 		log := logger.NewBuffer()
 		err := agentStartupHook(log, cfg(hooksPath))
 
@@ -94,6 +96,59 @@ func TestAgentStartupHook(t *testing.T) {
 	})
 }
 
+func TestAgentStartupHookWithAdditionalPaths(t *testing.T) {
+	t.SkipNow()
+	// This test was added to validate that multiple global hooks can be added
+	// by using the AdditionalHooksPaths configuration option. When this test
+	// runs however, there's a timing issue where the second hook errors at
+	// execution time as the file is not available.
+	//
+	// Error:          Received unexpected error:
+	//                 error running "/opt/homebrew/bin/bash /var/folders/x3/rsj92m015tdcby8gz2j_25ym0000gn/T/471662504/agent-startup": unexpected error type *errors.errorString: io: read/write on closed pipe
+	// Test:           TestAgentStartupHookWithAdditionalPaths/with_additional_agent-startup_hook
+	// Messages:       [[info] $ /var/folders/x3/rsj92m015tdcby8gz2j_25ym0000gn/T/982974833/agent-startup [info] hello new world [error] "agent-startup" hook: error running "/opt/homebrew/bin/bash /var/folders/x3/rsj92m015tdcby8gz2j_25ym0000gn/T/471662504/agent-startup": unexpected error type *errors.errorString: io: read/write on closed pipe]
+	//
+	// For now it is skipped, and left as a placeholder!
+
+	t.Parallel()
+
+	cfg := func(hooksPath, additionalHooksPath string) AgentStartConfig {
+		return AgentStartConfig{
+			HooksPath:            hooksPath,
+			AdditionalHooksPaths: []string{additionalHooksPath},
+			NoColor:              true,
+		}
+	}
+	prompt := "$"
+	if runtime.GOOS == "windows" {
+		prompt = ">"
+	}
+
+	t.Run("with additional agent-startup hook", func(t *testing.T) {
+		t.Parallel()
+
+		hooksPath, closer := setupHooksPath(t)
+		filepath := writeAgentHook(t, hooksPath, "agent-startup", "hello new world")
+		defer closer()
+
+		additionalHooksPath, additionalCloser := setupHooksPath(t)
+		addFilepath := writeAgentHook(t, additionalHooksPath, "agent-startup", "hello additional world")
+		defer additionalCloser()
+
+		log := logger.NewBuffer()
+		err := agentStartupHook(log, cfg(hooksPath, additionalHooksPath))
+
+		if assert.NoError(t, err, log.Messages) {
+			assert.Equal(t, []string{
+				"[info] " + prompt + " " + filepath,    // prompt
+				"[info] hello new world",               // output
+				"[info] " + prompt + " " + addFilepath, // prompt
+				"[info] hello additional world",        // output
+			}, log.Messages)
+		}
+	})
+}
+
 func TestAgentShutdownHook(t *testing.T) {
 	t.Parallel()
 
@@ -113,7 +168,7 @@ func TestAgentShutdownHook(t *testing.T) {
 
 		hooksPath, closer := setupHooksPath(t)
 		defer closer()
-		filepath := writeAgentHook(t, hooksPath, "agent-shutdown")
+		filepath := writeAgentHook(t, hooksPath, "agent-shutdown", "hello world")
 		log := logger.NewBuffer()
 		agentShutdownHook(log, cfg(hooksPath))
 

clicommand/bootstrap.go

@@ -75,6 +75,7 @@ type BootstrapConfig struct {
 	BinPath                      string   `cli:"bin-path" normalize:"filepath"`
 	BuildPath                    string   `cli:"build-path" normalize:"filepath"`
 	HooksPath                    string   `cli:"hooks-path" normalize:"filepath"`
+	AdditionalHooksPaths         []string `cli:"additional-hooks-paths" normalize:"list"`
 	SocketsPath                  string   `cli:"sockets-path" normalize:"filepath"`
 	PluginsPath                  string   `cli:"plugins-path" normalize:"filepath"`
 	CommandEval                  bool     `cli:"command-eval"`
@@ -280,6 +281,12 @@ var BootstrapCommand = cli.Command{
 			Usage:  "Directory where the hook scripts are found",
 			EnvVar: "BUILDKITE_HOOKS_PATH",
 		},
+		cli.StringSliceFlag{
+			Name:   "additional-hooks-paths",
+			Value:  &cli.StringSlice{},
+			Usage:  "Any additional directories to look for agent hooks",
+			EnvVar: "BUILDKITE_ADDITIONAL_HOOKS_PATHS",
+		},
 		cli.StringFlag{
 			Name:   "sockets-path",
 			Value:  defaultSocketsPath(),
@@ -449,6 +456,7 @@ var BootstrapCommand = cli.Command{
 			GitSubmodules:                cfg.GitSubmodules,
 			GitSubmoduleCloneConfig:      cfg.GitSubmoduleCloneConfig,
 			HooksPath:                    cfg.HooksPath,
+			AdditionalHooksPaths:         cfg.AdditionalHooksPaths,
 			JobID:                        cfg.JobID,
 			LocalHooksEnabled:            cfg.LocalHooksEnabled,
 			OrganizationSlug:             cfg.OrganizationSlug,

internal/job/config.go

@@ -132,6 +132,9 @@ type ExecutorConfig struct {
 	// Path to the global hooks
 	HooksPath string
 
+	// Additional hooks directories that can be provided
+	AdditionalHooksPaths []string
+
 	// Path to the plugins directory
 	PluginsPath string
 

internal/job/executor.go

@@ -654,29 +654,59 @@ func (e *Executor) applyEnvironmentChanges(changes hook.EnvChanges) {
 }
 
 func (e *Executor) hasGlobalHook(name string) bool {
-	_, err := e.globalHookPath(name)
-	return err == nil
+	_, err := hook.Find(e.HooksPath, name)
+	if err == nil {
+		return true
+	}
+	for _, additional := range e.AdditionalHooksPaths {
+		_, err := hook.Find(additional, name)
+		if err == nil {
+			return true
+		}
+	}
+	return false
 }
 
-// Returns the absolute path to a global hook, or os.ErrNotExist if none is found
-func (e *Executor) globalHookPath(name string) (string, error) {
-	return hook.Find(e.HooksPath, name)
+// find all matching paths for the specified hook
+func (e *Executor) getAllGlobalHookPaths(name string) ([]string, error) {
+	hooks := []string{}
+	p, err := hook.Find(e.HooksPath, name)
+	if err != nil {
+		if !os.IsNotExist(err) {
+			return []string{}, err
+		}
+	} else {
+		hooks = append(hooks, p)
+	}
+
+	for _, additional := range e.AdditionalHooksPaths {
+		p, err = hook.Find(additional, name)
+		// as this is an additional hook, don't fail if there's a problem here
+		if err == nil {
+			hooks = append(hooks, p)
+		}
+	}
+
+	return hooks, nil
 }
 
 // Executes a global hook if one exists
 func (e *Executor) executeGlobalHook(ctx context.Context, name string) error {
-	if !e.hasGlobalHook(name) {
+	allHooks, err := e.getAllGlobalHookPaths(name)
+	if err != nil {
 		return nil
 	}
-	p, err := e.globalHookPath(name)
-	if err != nil {
-		return err
+	for _, h := range allHooks {
+		err = e.executeHook(ctx, HookConfig{
+			Scope: "global",
+			Name:  name,
+			Path:  h,
+		})
+		if err != nil {
+			return err
+		}
 	}
-	return e.executeHook(ctx, HookConfig{
-		Scope: "global",
-		Name:  name,
-		Path:  p,
-	})
+	return nil
 }
 
 // Returns the absolute path to a local hook, or os.ErrNotExist if none is found

packaging/docker/alpine-k8s/buildkite-agent.cfg

@@ -36,6 +36,9 @@ build-path="/buildkite/builds"
 # Directory where the hook scripts are found
 hooks-path="/buildkite/hooks"
 
+# Any additional directories where hook scripts are found
+# additional-hooks-path="/hook/path/a,/hook/path/b"
+
 # Directory where plugins will be installed
 plugins-path="/buildkite/plugins"
 

packaging/docker/alpine/buildkite-agent.cfg

@@ -36,6 +36,9 @@ build-path="/buildkite/builds"
 # Directory where the hook scripts are found
 hooks-path="/buildkite/hooks"
 
+# Any additional directories where hook scripts are found
+# additional-hooks-path="/hook/path/a,/hook/path/b"
+
 # Directory where plugins will be installed
 plugins-path="/buildkite/plugins"
 

packaging/docker/sidecar/buildkite-agent.cfg

@@ -36,6 +36,9 @@ build-path="/buildkite/builds"
 # Directory where the hook scripts are found
 hooks-path="/buildkite/hooks"
 
+# Any additional directories where hook scripts are found
+# additional-hooks-path="/hook/path/a,/hook/path/b"
+
 # Directory where plugins will be installed
 plugins-path="/buildkite/plugins"
 

packaging/docker/ubuntu-20.04/buildkite-agent.cfg

@@ -36,6 +36,9 @@ build-path="/buildkite/builds"
 # Directory where the hook scripts are found
 hooks-path="/buildkite/hooks"
 
+# Any additional directories where hook scripts are found
+# additional-hooks-path="/hook/path/a,/hook/path/b"
+
 # Directory where plugins will be installed
 plugins-path="/buildkite/plugins"
 

packaging/docker/ubuntu-22.04/buildkite-agent.cfg

@@ -36,6 +36,9 @@ build-path="/buildkite/builds"
 # Directory where the hook scripts are found
 hooks-path="/buildkite/hooks"
 
+# Any additional directories where hook scripts are found
+# additional-hooks-path="/hook/path/a,/hook/path/b"
+
 # Directory where plugins will be installed
 plugins-path="/buildkite/plugins"