buildkite-agent env has --from-env-file and --print flags

[pda]

Adds two flags to the buildkite-agent env command introduced in #1781.

By default, buildkite-agent env loads os.Environ() and prints it as a JSON object; it's mainly intended for internal use to capture a snapshot of environment before a hook runs, and detect changes afterwards.

The new --from-env-file boolean flag sources the environment from the file named by $BUILDKITE_ENV_FILE instead of from os.Environ(), using bufio.Scanner and strconv.Unquote() to safely and correctly parse the quoting/escaping done by %q when the file was written:

agent/agent/job_runner.go

Line 490 in 2aaefb6

if _, err := r.envFile.WriteString(fmt.Sprintf("%s=%q\n", key, value)); err != nil {

The new --print NAME flag causes a single environment value to be printed in its raw unescaped form followed by a newline, instead of printing all vars as a JSON object.

Combined, this solves the problem of safely parsing BUILDKITE_ENV_FILE (which cannot be safely evaluated in a shell if it may contain untrusted user input) to access a single variable, and allows for usage like this in a pre-command hook or similar:

if [[ $(buildkite-agent env --from-env-file --print BUILDKITE_BRANCH) != "main" ]]; then
  echo "This agent only builds branch 'main'"
  exit 42
fi

[triarius]

LGTM, but someone will likely have merge conficts.

[triarius]

I suspect @moskyb's efforts to also use this command in windows will cause a merge conflict here.

[sj26]

This feels weird to me. I expect --from-env-file to take a value, but default to an environment variable, like our other options. But if we defaulted to the env file then it would break dumping of the current env as json.

[pda]

Yep. I like your idea of moving the current “dump entire process env as JSON” use-cases into a subcommand/flag(s), which then frees up the buildkite-agent env namespace to support some other environment-related use-cases.

[sj26]

This feels like the same domain but a pretty orthogonal use case to the underlying command to dump the current env as json. I think it should be a separate command. Maybe something like:

buildkite-agent env get NAME

But it's unclear to me then which env it's getting. Build env? Job env? The current process? The env file, pointed at by an env (inception moment)? I would presume the current process env. Which feels odd, because I can $BLAH.

Playing with some usage ...

buildkite-agent env get NAME # current process env?
buildkite-agent env --env-file get NAME # from $BUILDKITE_ENV_FILE?
buildkite-agent env --env-file=... get NAME
buildkite-agent env --json-file=... get NAME

???

Maybe the recently introduced command should also be a subcommand?

buildkite-agent env dump --json

[pda]

Maybe the recently introduced command should also be a subcommand?

buildkite-agent env dump --json

Yeah I like that. Since the recently-added buildkite-agent env is private/internal usage only (hopefully), we're free to evolve it to support other use-cases around it in a coherent way 👍🏼

[sj26]

The nice thing about subcommands is if we peer forwards to being able to set env from things which are not bash hooks, that could use buildkite-agent env set NAME VALUE to talk to the parent process and inject env without sourcing and env dumping shenanigans at all.