buildkite / agent

The Buildkite Agent is an open-source toolkit written in Go for securely running build jobs on any device or network
https://buildkite.com/
MIT License
810 stars 300 forks source link

Run Python/Ruby scripts for hooks instead of Bash #1136

Open evandam opened 4 years ago

evandam commented 4 years ago

All the example plugins I see are 100% bash scripts, but it would be really useful to break into another language like Python or Ruby to be more productive.

I tried making a plugin with a simple Python script but see things like this, where it's running each line as bash rather than respecting the hook file's shebang:

/var/lib/buildkite-agent/builds/default-ubuntu-1804-vagrantup-com-1/my-org/my-repo/.buildkite/plugins/my-plugin/hooks/pre-command: line 3: import: command not found
--

I set BUILDKITE_SHELL to /bin/bash -eo pipefail -c, which seems pretty close to the default.

@lox any thoughts? :smile:

matthewd commented 4 years ago

There's a related feature request over here: https://forum.buildkite.community/t/native-support-for-golang-plugins/776

The fundamental reason it doesn't/can't "just work" is that hooks are sourced, not forked & executed -- which is necessary so that changes to environment variables can be detected and propagated to subsequent hooks / the command environment.

For hooks that don't need to export anything like that, you can work around the current behaviour by using a one-liner hook shell script that just executes your "real" script... but I think inter-hook environment variables are a common enough use case that we'd need a stronger solution before we could change the general approach.

Do you have any thoughts on a good way to solve the env-from-python/ruby problem? Or know of any interesting prior art?

evandam commented 4 years ago

Good point around being able to export/propagate environment variables set in the hooks. I was able to do a one-liner, but it wasn't immediately obvious how to do so:

$(dirname ${BASH_SOURCE})}/../scripts/pre-command.py

I'm not sure if there's a better way to handle this, since $0 doesn't seem to be set as expected, presumably due to the way the script is sourced, and we're assuming the shell is always bash.

I'm okay with this kind of solution, but could there be some documentation around the recommended way to make this kind of hook?

dbaggerman commented 4 years ago

Looks like the agent generates a small wrapper script on the fly to load the hook script and write out the environment before/after. It's possible to do the same for both python and ruby.

Python can be handled by creating a wrapper and using the runpy module:

import os
import runpy

with open('env-before', 'w') as f:
    for (key, val) in os.environ.items():
        f.write(f"{key}={val}")

runpy.run_path('/path/to/hook.py', run_name = '__main__')

with open('env-after', 'w') as f:
    for (key, val) in os.environ.items():
        f.write(f"{key}={val}")

Ruby is about the same:

open('env-before', 'w') do |f|
  f.puts ENV.map {|k,v| "#{k}=#{v}" }
end

$0 = '/path/to/hook.rb'
load '/path/to/hook.rb'

open('env-after', 'w') do |f|
  f.puts ENV.map {|k,v| "#{k}=#{v}" }
end
dbaggerman commented 4 years ago

Golang, as per the forum thread is a bit trickier. You could require people build their hooks as plugins (https://golang.org/pkg/plugin/) and load them from the agent bootstrap process, but the lack of Windows support might be a concern.

evandam commented 4 years ago

I'm a bit confused, is the Python/Ruby approach something that would be possible, or is it more or less what Buildkite is doing for us automatically with its wrapper script?

I think kicking off python/ruby/go from a shell script is perfectly fine, but there just isn't much documentation on how to do it besides some trial/error around messing with $0, $BASH_SOURCE, and figuring out the pwd of each hook.

dbaggerman commented 4 years ago

@evandam, my comments were directed at @matthewd and the Buildkite team. It would be possible for them to update the agent to support python or ruby hooks.

I wasn't suggesting this as something you can do. Sorry if it wasn't clear.

lox commented 4 years ago

The other option would be to not support capturing changed environment for non-shell-script hooks

lox commented 4 years ago

IMO that would be a fairly reasonable compromise.

dbaggerman commented 4 years ago

The other option would be to not support capturing changed environment for non-shell-script hooks

That should work for most cases. It would be nice to at least be able to handle exporting environment variables manually. I'm thinking something like a WRITE_ENVS_HERE environment variable with a file path for the hook to write to explicitly.

If you wanted to go down the route of requiring explicit actions from the hooks, I'd also suggest a BUILDKITE_PHASE environment variable to support the "all hooks in one codebase/entrypoint" idea.

keithpitt commented 4 years ago

For the record, I'd love to be able to get this working. So if there's something we can do (but perhaps with compromises as @lox pointed out) I think that'd be rad.

Another idea is that make a temple and provide it as ENV_CHANGES or some such, and folks can write to that file with env they want to export?

keithpitt commented 4 years ago

Ha ha, sorry, my brain failed. Pretty much what @dbaggerman said :)

kevjin commented 4 years ago

+1 for this feature :)