Experiment for propagating agent config env vars#3471
Conversation
There was a problem hiding this comment.
The code change looks safe so LGTM, but @DrJosh9000 do you mind giving me some context on what use cases are this env file designed for?
I did see #3430 but I don't quite know how env file is used.
Mykematt
left a comment
There was a problem hiding this comment.
Thanks @DrJosh9000 for this. But from my test, even with experiment enabled, new vars does not work when propagate-environment: true is enabled, and this is precisely what we set out to fix.
Consequently, I added the below to agent/job_runner.go:
// Agent configuration variables - set unconditionally for backward compatibility
// Git configuration variables
if r.conf.AgentConfiguration.GitCheckoutFlags != "" {
env["BUILDKITE_GIT_CHECKOUT_FLAGS"] = r.conf.AgentConfiguration.GitCheckoutFlags
}
if r.conf.AgentConfiguration.GitCleanFlags != "" {
env["BUILDKITE_GIT_CLEAN_FLAGS"] = r.conf.AgentConfiguration.GitCleanFlags
}
// ... (continue for all 32 agent config variables)
// Tracing variables
if r.conf.AgentConfiguration.TraceContextEncoding != "" {
env["BUILDKITE_TRACE_CONTEXT_ENCODING"] = r.conf.AgentConfiguration.TraceContextEncoding
}
if r.conf.AgentConfiguration.TracingBackend != "" {
env["BUILDKITE_TRACING_BACKEND"] = r.conf.AgentConfiguration.TracingBackend
env["BUILDKITE_TRACING_SERVICE_NAME"] = r.conf.AgentConfiguration.TracingServiceName
// ... (continue for tracing variables)
}
// Pipeline signing variables
if r.conf.AgentConfiguration.SigningAWSKMSKey != "" {
env["BUILDKITE_AGENT_AWS_KMS_KEY"] = r.conf.AgentConfiguration.SigningAWSKMSKey
}
// ... (continue for signing variables)This ensured that in addition to writing variable names to env file they also exist in agent environment. The latter allowed Docker plugins with propagate-environment: true to read from agent environment, not just env file. Without this, variables were missing even when experiment is enabled.
Building a custom agent with this, tests reveal that propagate-environment: true fetched new vars when experiment is enabled.
|
Thanks @Mykematt, but can you explain why those env vars are not in the environment readable by the plugin when similar lines of code already exist in
Can you also explain why the two tests you linked used different versions of the docker-compose plugin? (v4.16.0 vs v5.3.0)? |
|
@Mykematt I've done some further testing, which shows the env vars are indeed propagating. Without the experiment: https://buildkite.com/stargoose/propagate-agent-config-vars/builds/31/steps/canvas?sid=01994bb7-9067-4569-aa85-aebb8845fc0b With the experiment: https://buildkite.com/stargoose/propagate-agent-config-vars/builds/32/steps/canvas?sid=01994bb8-120f-459b-89c6-149082061c65 Note without the experiment the absence of |
The env file is used by pre-bootstrap hooks to allow custom job validation before running anything else. The env vars are passed via a file specifically to avoid shell injection. See e.g. https://buildkite.com/docs/agent/v3/securing#restrict-access-by-the-buildkite-agent-controller-strict-checks-using-a-pre-bootstrap-hook. Because the format of the file has some quirks, we also write a JSON version, but there will be a few uses of the original env file that I am wary of breaking, given how long it has been in the docs. Hence the need for an experiment (or a different file). The env file is also used by the docker and docker-compose plugins to figure out what |
|
@DrJosh9000 thanks once again. Looking through your tests, I may have overthought somethings while testing. Your workaround solves what I set out to achieve. As for the different docker-compose plugin versions, it was an oversight. Ran several tests and possibly confused things there a bit. Thanks once again for taking a crack at this. I will drop a note in the associated linear allowing us to close it. |
|
@Mykematt And thank you for persisting, agent stuff can be complicated! |
Description
Add an experiment that enables various agent config vars to be named in the env file.
Context
Closes #3430
Changes
propagate-agent-config-varsTesting
go test ./...). Buildkite employees may check this if the pipeline has run automatically.go fmt ./...)Disclosures / Credits
I did not use AI tools at all. I copied the list of env vars from #3430.