From 6023d831cbf0d17b8e58cbf840567adfd4afa848 Mon Sep 17 00:00:00 2001 From: Josh Deprez Date: Thu, 12 Sep 2024 15:21:18 +1000 Subject: [PATCH 1/5] Add JSON-format env file --- agent/job_runner.go | 51 ++++++++++++++++++++++++++++++++++----------- agent/run_job.go | 10 ++++++--- 2 files changed, 46 insertions(+), 15 deletions(-) diff --git a/agent/job_runner.go b/agent/job_runner.go index 44662bd022..edf9fdd868 100644 --- a/agent/job_runner.go +++ b/agent/job_runner.go @@ -2,6 +2,7 @@ package agent import ( "context" + "encoding/json" "fmt" "io" "os" @@ -156,8 +157,9 @@ type JobRunner struct { // A lock to protect concurrent calls to cancel cancelLock sync.Mutex - // File containing a copy of the job env - envFile *os.File + // Files containing a copy of the job env + envShellFile *os.File + envJSONFile *os.File } type jobAPI interface { @@ -225,12 +227,19 @@ func NewJobRunner(ctx context.Context, l logger.Logger, apiClient APIClient, con } // Prepare a file to receive the given job environment - if file, err := os.CreateTemp(tempDir, fmt.Sprintf("job-env-%s", r.conf.Job.ID)); err != nil { + file, err := os.CreateTemp(tempDir, fmt.Sprintf("job-env-%s", r.conf.Job.ID)) + if err != nil { + return r, err + } + r.agentLogger.Debug("[JobRunner] Created env file (shell format): %s", file.Name()) + r.envShellFile = file + + file, err = os.CreateTemp(tempDir, fmt.Sprintf("job-env-json-%s", r.conf.Job.ID)) + if err != nil { return r, err - } else { - r.agentLogger.Debug("[JobRunner] Created env file: %s", file.Name()) - r.envFile = file } + r.agentLogger.Debug("[JobRunner] Created env file (JSON format): %s", file.Name()) + r.envJSONFile = file env, err := r.createEnvironment(ctx) if err != nil { @@ -420,19 +429,36 @@ func (r *JobRunner) createEnvironment(ctx context.Context) ([]string, error) { // The agent registration token should never make it into the job environment delete(env, "BUILDKITE_AGENT_TOKEN") - // Write out the job environment to a file, in k="v" format, with newlines escaped + // Write out the job environment to file: + // - envShellFile: in k="v" format, with newlines escaped + // - envJSONFile: as a single JSON object {"k":"v",...}, escaped appropriately for JSON. // We present only the clean environment - i.e only variables configured // on the job upstream - and expose the path in another environment variable. - if r.envFile != nil { + if r.envShellFile != nil { for key, value := range env { - if _, err := r.envFile.WriteString(fmt.Sprintf("%s=%q\n", key, value)); err != nil { + if _, err := r.envShellFile.WriteString(fmt.Sprintf("%s=%q\n", key, value)); err != nil { return nil, err } } - if err := r.envFile.Close(); err != nil { + if err := r.envShellFile.Close(); err != nil { + return nil, err + } + } + if r.envJSONFile != nil { + if err := json.NewEncoder(r.envJSONFile).Encode(env); err != nil { + return nil, err + } + if err := r.envJSONFile.Close(); err != nil { return nil, err } - env["BUILDKITE_ENV_FILE"] = r.envFile.Name() + } + // Now that the env files have been written, we can add their corresponding + // paths to the job env. + if r.envShellFile != nil { + env["BUILDKITE_ENV_FILE"] = r.envShellFile.Name() + } + if r.envJSONFile != nil { + env["BUILDKITE_ENV_JSON_FILE"] = r.envJSONFile.Name() } var ignoredEnv []string @@ -613,7 +639,8 @@ func (r *JobRunner) executePreBootstrapHook(ctx context.Context, hook string) (b // This (plus inherited) is the only ENV that should be exposed // to the pre-bootstrap hook. - sh.Env.Set("BUILDKITE_ENV_FILE", r.envFile.Name()) + sh.Env.Set("BUILDKITE_ENV_FILE", r.envShellFile.Name()) + sh.Env.Set("BUILDKITE_ENV_JSON_FILE", r.envJSONFile.Name()) sh.Writer = LogWriter{ l: r.agentLogger, diff --git a/agent/run_job.go b/agent/run_job.go index 6caa3ae92d..a78fc659ea 100644 --- a/agent/run_job.go +++ b/agent/run_job.go @@ -349,11 +349,15 @@ func (r *JobRunner) cleanup(ctx context.Context, wg *sync.WaitGroup, exit core.P wg.Wait() // Remove the env file, if any - if r.envFile != nil { - if err := os.Remove(r.envFile.Name()); err != nil { + for _, f := range []*os.File{r.envShellFile, r.envJSONFile} { + if f == nil { + continue + } + if err := os.Remove(f.Name()); err != nil { r.agentLogger.Warn("[JobRunner] Error cleaning up env file: %s", err) + continue } - r.agentLogger.Debug("[JobRunner] Deleted env file: %s", r.envFile.Name()) + r.agentLogger.Debug("[JobRunner] Deleted env file: %s", f.Name()) } // Write some metrics about the job run From 19f33be6c82536794410c780e5438eb907cee262 Mon Sep 17 00:00:00 2001 From: Josh Deprez Date: Thu, 12 Sep 2024 15:21:18 +1000 Subject: [PATCH 2/5] Tweaks to job_runner_test.go * Better regular expressions * Test style improvements --- agent/job_runner_test.go | 54 +++++++++++++++++++++++----------------- 1 file changed, 31 insertions(+), 23 deletions(-) diff --git a/agent/job_runner_test.go b/agent/job_runner_test.go index 4fb346b7c5..83ee3d8ef9 100644 --- a/agent/job_runner_test.go +++ b/agent/job_runner_test.go @@ -7,43 +7,51 @@ import ( "testing" "github.com/buildkite/agent/v3/logger" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) func TestTruncateEnv(t *testing.T) { l := logger.NewBuffer() - env := map[string]string{"FOO": strings.Repeat("a", 100)} - err := truncateEnv(l, env, "FOO", 64) - require.NoError(t, err) - assert.Equal(t, "aaaaaaaaaaaaaaaaaaaaaaaaaa[value truncated 100 -> 59 bytes]", env["FOO"]) - assert.Equal(t, 64, len(fmt.Sprintf("FOO=%s\000", env["FOO"]))) + key := "FOO" + env := map[string]string{key: strings.Repeat("a", 100)} + limit := 64 + if err := truncateEnv(l, env, key, limit); err != nil { + t.Fatalf("truncateEnv(logger, %v, %q, %d) = %v", env, key, limit, err) + } + if got, want := env["FOO"], "aaaaaaaaaaaaaaaaaaaaaaaaaa[value truncated 100 -> 59 bytes]"; got != want { + t.Errorf("after truncateEnv(logger, %v, %q, %d): env[%q] = %q, want %q", env, key, limit, key, got, want) + } + format := "FOO=%s\000" + if got, want := len(fmt.Sprintf(format, env["FOO"])), limit; got != want { + t.Errorf("after truncateEnv(logger, %v, %q, %d): len(fmt.Sprintf(%q, env[%q])) = %d, want %d", env, key, limit, format, key, got, want) + } } func TestValidateJobValue(t *testing.T) { bkTarget := "github.com/buildkite/test" - bkTargetRe := regexp.MustCompile("^github.com/buildkite/.*") - ghTargetRe := regexp.MustCompile("^github.com/nope/.*") + bkTargetRE := regexp.MustCompile(`^github\.com/buildkite/.*`) + ghTargetRE := regexp.MustCompile(`^github\.com/nope/.*`) tests := []struct { name string allowedTargets []*regexp.Regexp pipelineTarget string wantErr bool - }{{ - name: "No error. Allowed targets no configured.", - allowedTargets: []*regexp.Regexp{}, - pipelineTarget: bkTarget, - }, { - name: "No pipeline target match", - allowedTargets: []*regexp.Regexp{ghTargetRe}, - pipelineTarget: bkTarget, - wantErr: true, - }, { - name: "Pipeline target match", - allowedTargets: []*regexp.Regexp{ghTargetRe, bkTargetRe}, - pipelineTarget: bkTarget, - }} + }{ + { + name: "No error. Allowed targets no configured.", + allowedTargets: []*regexp.Regexp{}, + pipelineTarget: bkTarget, + }, { + name: "No pipeline target match", + allowedTargets: []*regexp.Regexp{ghTargetRE}, + pipelineTarget: bkTarget, + wantErr: true, + }, { + name: "Pipeline target match", + allowedTargets: []*regexp.Regexp{ghTargetRE, bkTargetRE}, + pipelineTarget: bkTarget, + }, + } for _, tc := range tests { err := validateJobValue(tc.allowedTargets, tc.pipelineTarget) From 9d727203d3602f1f8b8c2b7bd0c4aaa9c4d84be0 Mon Sep 17 00:00:00 2001 From: Josh Deprez Date: Thu, 12 Sep 2024 17:00:49 +1000 Subject: [PATCH 3/5] Allow pre-bootstrap hook to annotate --- agent/job_runner.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/agent/job_runner.go b/agent/job_runner.go index edf9fdd868..6a2358f6b2 100644 --- a/agent/job_runner.go +++ b/agent/job_runner.go @@ -639,8 +639,18 @@ func (r *JobRunner) executePreBootstrapHook(ctx context.Context, hook string) (b // This (plus inherited) is the only ENV that should be exposed // to the pre-bootstrap hook. + // - Env files are designed to be validated by the pre-bootstrap hook + // - The pre-bootstrap hook may want to create annotations, so it can also + // have a few necessary and global args as env vars. sh.Env.Set("BUILDKITE_ENV_FILE", r.envShellFile.Name()) sh.Env.Set("BUILDKITE_ENV_JSON_FILE", r.envJSONFile.Name()) + apiConfig := r.apiClient.Config() + sh.Env.Set("BUILDKITE_JOB_ID", r.conf.Job.ID) + sh.Env.Set("BUILDKITE_AGENT_ACCESS_TOKEN", apiConfig.Token) + sh.Env.Set("BUILDKITE_AGENT_ENDPOINT", apiConfig.Endpoint) + sh.Env.Set("BUILDKITE_NO_HTTP2", fmt.Sprint(apiConfig.DisableHTTP2)) + sh.Env.Set("BUILDKITE_AGENT_DEBUG", fmt.Sprint(r.conf.Debug)) + sh.Env.Set("BUILDKITE_AGENT_DEBUG_HTTP", fmt.Sprint(r.conf.DebugHTTP)) sh.Writer = LogWriter{ l: r.agentLogger, From 9225a45c57d991061e650b53f5efca2345adcf47 Mon Sep 17 00:00:00 2001 From: Josh Deprez Date: Thu, 12 Sep 2024 17:00:49 +1000 Subject: [PATCH 4/5] fmt.Sprintf("%t", b) -> fmt.Sprint(b) --- agent/job_runner.go | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/agent/job_runner.go b/agent/job_runner.go index 6a2358f6b2..37b4ce3cee 100644 --- a/agent/job_runner.go +++ b/agent/job_runner.go @@ -484,10 +484,11 @@ func (r *JobRunner) createEnvironment(ctx context.Context) ([]string, error) { apiConfig := r.apiClient.Config() env["BUILDKITE_AGENT_ENDPOINT"] = apiConfig.Endpoint env["BUILDKITE_AGENT_ACCESS_TOKEN"] = apiConfig.Token + env["BUILDKITE_NO_HTTP2"] = fmt.Sprint(apiConfig.DisableHTTP2) // Add agent environment variables - env["BUILDKITE_AGENT_DEBUG"] = fmt.Sprintf("%t", r.conf.Debug) - env["BUILDKITE_AGENT_DEBUG_HTTP"] = fmt.Sprintf("%t", r.conf.DebugHTTP) + env["BUILDKITE_AGENT_DEBUG"] = fmt.Sprint(r.conf.Debug) + env["BUILDKITE_AGENT_DEBUG_HTTP"] = fmt.Sprint(r.conf.DebugHTTP) env["BUILDKITE_AGENT_PID"] = strconv.Itoa(os.Getpid()) // We know the BUILDKITE_BIN_PATH dir, because it's the path to the @@ -507,14 +508,14 @@ func (r *JobRunner) createEnvironment(ctx context.Context) ([]string, error) { env["BUILDKITE_BUILD_PATH"] = r.conf.AgentConfiguration.BuildPath env["BUILDKITE_SOCKETS_PATH"] = r.conf.AgentConfiguration.SocketsPath env["BUILDKITE_GIT_MIRRORS_PATH"] = r.conf.AgentConfiguration.GitMirrorsPath - env["BUILDKITE_GIT_MIRRORS_SKIP_UPDATE"] = fmt.Sprintf("%t", r.conf.AgentConfiguration.GitMirrorsSkipUpdate) + env["BUILDKITE_GIT_MIRRORS_SKIP_UPDATE"] = fmt.Sprint(r.conf.AgentConfiguration.GitMirrorsSkipUpdate) env["BUILDKITE_HOOKS_PATH"] = r.conf.AgentConfiguration.HooksPath env["BUILDKITE_PLUGINS_PATH"] = r.conf.AgentConfiguration.PluginsPath - env["BUILDKITE_SSH_KEYSCAN"] = fmt.Sprintf("%t", r.conf.AgentConfiguration.SSHKeyscan) - env["BUILDKITE_GIT_SUBMODULES"] = fmt.Sprintf("%t", r.conf.AgentConfiguration.GitSubmodules) - env["BUILDKITE_COMMAND_EVAL"] = fmt.Sprintf("%t", r.conf.AgentConfiguration.CommandEval) - env["BUILDKITE_PLUGINS_ENABLED"] = fmt.Sprintf("%t", r.conf.AgentConfiguration.PluginsEnabled) - env["BUILDKITE_LOCAL_HOOKS_ENABLED"] = fmt.Sprintf("%t", r.conf.AgentConfiguration.LocalHooksEnabled) + env["BUILDKITE_SSH_KEYSCAN"] = fmt.Sprint(r.conf.AgentConfiguration.SSHKeyscan) + env["BUILDKITE_GIT_SUBMODULES"] = fmt.Sprint(r.conf.AgentConfiguration.GitSubmodules) + env["BUILDKITE_COMMAND_EVAL"] = fmt.Sprint(r.conf.AgentConfiguration.CommandEval) + env["BUILDKITE_PLUGINS_ENABLED"] = fmt.Sprint(r.conf.AgentConfiguration.PluginsEnabled) + env["BUILDKITE_LOCAL_HOOKS_ENABLED"] = fmt.Sprint(r.conf.AgentConfiguration.LocalHooksEnabled) env["BUILDKITE_GIT_CHECKOUT_FLAGS"] = r.conf.AgentConfiguration.GitCheckoutFlags env["BUILDKITE_GIT_CLONE_FLAGS"] = r.conf.AgentConfiguration.GitCloneFlags env["BUILDKITE_GIT_FETCH_FLAGS"] = r.conf.AgentConfiguration.GitFetchFlags @@ -524,7 +525,7 @@ func (r *JobRunner) createEnvironment(ctx context.Context) ([]string, error) { env["BUILDKITE_SHELL"] = r.conf.AgentConfiguration.Shell env["BUILDKITE_AGENT_EXPERIMENT"] = strings.Join(experiments.Enabled(ctx), ",") env["BUILDKITE_REDACTED_VARS"] = strings.Join(r.conf.AgentConfiguration.RedactedVars, ",") - env["BUILDKITE_STRICT_SINGLE_HOOKS"] = fmt.Sprintf("%t", r.conf.AgentConfiguration.StrictSingleHooks) + env["BUILDKITE_STRICT_SINGLE_HOOKS"] = fmt.Sprint(r.conf.AgentConfiguration.StrictSingleHooks) env["BUILDKITE_CANCEL_GRACE_PERIOD"] = strconv.Itoa(r.conf.AgentConfiguration.CancelGracePeriod) env["BUILDKITE_SIGNAL_GRACE_PERIOD_SECONDS"] = strconv.Itoa(int(r.conf.AgentConfiguration.SignalGracePeriod / time.Second)) env["BUILDKITE_TRACE_CONTEXT_ENCODING"] = r.conf.AgentConfiguration.TraceContextEncoding @@ -576,7 +577,7 @@ func (r *JobRunner) createEnvironment(ctx context.Context) ([]string, error) { enablePluginValidation = true } } - env["BUILDKITE_PLUGIN_VALIDATION"] = fmt.Sprintf("%t", enablePluginValidation) + env["BUILDKITE_PLUGIN_VALIDATION"] = fmt.Sprint(enablePluginValidation) if r.conf.AgentConfiguration.TracingBackend != "" { env["BUILDKITE_TRACING_BACKEND"] = r.conf.AgentConfiguration.TracingBackend From 3c9f0c02772d80326fe5c065e0ccf4e1f6b2f781 Mon Sep 17 00:00:00 2001 From: Josh Deprez Date: Mon, 16 Sep 2024 10:10:47 +1000 Subject: [PATCH 5/5] go generate ./... --- internal/mime/mime.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/mime/mime.go b/internal/mime/mime.go index 0e686a0822..2c428e89b7 100644 --- a/internal/mime/mime.go +++ b/internal/mime/mime.go @@ -388,6 +388,7 @@ var types = map[string]string{ ".js": "application/javascript", ".json": "application/json", ".jsonml": "application/jsonml+json", + ".jxl": "image/jxl", ".kar": "audio/midi", ".karbon": "application/vnd.kde.karbon", ".kfo": "application/vnd.kde.kformula",