diff --git a/AGENT.md b/AGENT.md index b24bc6c5a4..0774fdfec7 100644 --- a/AGENT.md +++ b/AGENT.md @@ -28,7 +28,7 @@ Go CLI application with main packages: - Formatting with `gofumpt` in extra mode: `go tool gofumpt -extra -w .` - Struct-based configuration patterns (e.g., `AgentWorkerConfig`, `JobRunnerConfig`) - Context-aware functions: `func Name(ctx context.Context, ...)` -- Import organization: stdlib, external deps, internal packages +- Import organization: stdlib, then everything else (gofumpt groups all non-stdlib imports together) - Error handling: explicit errors, wrapped with context - Naming: PascalCase for exported, camelCase for private, ALL_CAPS for constants - Interface types end with -er suffix where appropriate diff --git a/api/meta_data.go b/api/meta_data.go index a9a591320b..ceff7d3751 100644 --- a/api/meta_data.go +++ b/api/meta_data.go @@ -18,6 +18,11 @@ type MetaDataExists struct { Exists bool `json:"exists"` } +// MetaDataBatch represents a batch of key/value pairs for the set-batch endpoint. +type MetaDataBatch struct { + Items []MetaData `json:"items"` +} + // Sets the meta data value func (c *Client) SetMetaData(ctx context.Context, jobId string, metaData *MetaData) (*Response, error) { u := fmt.Sprintf("jobs/%s/data/set", railsPathEscape(jobId)) @@ -30,6 +35,19 @@ func (c *Client) SetMetaData(ctx context.Context, jobId string, metaData *MetaDa return c.doRequest(req, nil) } +// SetMetaDataBatch sets multiple meta data key/value pairs in a single request. +// The operation is transactional: all items succeed or none do. +func (c *Client) SetMetaDataBatch(ctx context.Context, jobId string, batch *MetaDataBatch) (*Response, error) { + u := fmt.Sprintf("jobs/%s/data/set-batch", railsPathEscape(jobId)) + + req, err := c.newRequest(ctx, "POST", u, batch) + if err != nil { + return nil, err + } + + return c.doRequest(req, nil) +} + // Gets the meta data value func (c *Client) GetMetaData(ctx context.Context, scope, id, key string) (*MetaData, *Response, error) { if scope != "job" && scope != "build" { diff --git a/clicommand/BUILD.bazel b/clicommand/BUILD.bazel index 77557d4787..55ee819b1d 100644 --- a/clicommand/BUILD.bazel +++ b/clicommand/BUILD.bazel @@ -41,6 +41,7 @@ go_library( "meta_data_get.go", "meta_data_keys.go", "meta_data_set.go", + "meta_data_set_batch.go", "oidc_request_token.go", "pipeline_upload.go", "profiler.go", @@ -118,6 +119,7 @@ go_test( "artifact_shasum_test.go", "build_cancel_test.go", "config_completeness_test.go", + "meta_data_set_batch_test.go", "git_credentials_helper_test.go", "pipeline_upload_test.go", "redactor_add_test.go", @@ -125,6 +127,7 @@ go_test( ], embed = [":clicommand"], deps = [ + "//api", "//core", "//env", "//internal/experiments", diff --git a/clicommand/commands.go b/clicommand/commands.go index 80cf7f3a1d..cb81a46c91 100644 --- a/clicommand/commands.go +++ b/clicommand/commands.go @@ -99,6 +99,7 @@ var BuildkiteAgentCommands = []cli.Command{ Usage: "Get/set metadata from Buildkite jobs", Subcommands: []cli.Command{ MetaDataSetCommand, + MetaDataSetBatchCommand, MetaDataGetCommand, MetaDataExistsCommand, MetaDataKeysCommand, diff --git a/clicommand/config_completeness_test.go b/clicommand/config_completeness_test.go index 238cbb82c9..ef4a7a20ba 100644 --- a/clicommand/config_completeness_test.go +++ b/clicommand/config_completeness_test.go @@ -45,6 +45,7 @@ var commandConfigPairs = []configCommandPair{ {Config: MetaDataGetConfig{}, Command: MetaDataGetCommand}, {Config: MetaDataKeysConfig{}, Command: MetaDataKeysCommand}, {Config: MetaDataSetConfig{}, Command: MetaDataSetCommand}, + {Config: MetaDataSetBatchConfig{}, Command: MetaDataSetBatchCommand}, {Config: OIDCTokenConfig{}, Command: OIDCRequestTokenCommand}, {Config: PipelineUploadConfig{}, Command: PipelineUploadCommand}, {Config: RedactorAddConfig{}, Command: RedactorAddCommand}, diff --git a/clicommand/meta_data_set_batch.go b/clicommand/meta_data_set_batch.go new file mode 100644 index 0000000000..ecf22bad1f --- /dev/null +++ b/clicommand/meta_data_set_batch.go @@ -0,0 +1,129 @@ +package clicommand + +import ( + "context" + "errors" + "fmt" + "slices" + "strings" + "time" + + "github.com/buildkite/agent/v3/api" + "github.com/buildkite/agent/v3/internal/redact" + "github.com/buildkite/agent/v3/logger" + "github.com/buildkite/roko" + "github.com/urfave/cli" +) + +const metaDataSetBatchHelpDescription = `Usage: + + buildkite-agent meta-data set-batch ... [options...] + +Description: + +Set multiple meta-data key/value pairs on a build in a single request. + +Each argument must be in key=value format. + +Keys and values must be non-empty strings, and strings containing only +whitespace characters are not allowed. + +Example: + + $ buildkite-agent meta-data set-batch foo=bar "greeting=hello world" + $ buildkite-agent meta-data set-batch duration:spec/a.rb=2.341 duration:spec/b.rb=5.672` + +type MetaDataSetBatchConfig struct { + GlobalConfig + APIConfig + + Job string `cli:"job" validate:"required"` + RedactedVars []string `cli:"redacted-vars" normalize:"list"` +} + +var MetaDataSetBatchCommand = cli.Command{ + Name: "set-batch", + Usage: "Set multiple meta-data key/value pairs on a build", + Description: metaDataSetBatchHelpDescription, + Flags: slices.Concat(globalFlags(), apiFlags(), []cli.Flag{ + cli.StringFlag{ + Name: "job", + Value: "", + Usage: "Which job's build should the meta-data be set on", + EnvVar: "BUILDKITE_JOB_ID", + }, + RedactedVars, + }), + Action: func(c *cli.Context) error { + ctx := context.Background() + ctx, cfg, l, _, done := setupLoggerAndConfig[MetaDataSetBatchConfig](ctx, c) + defer done() + + args := c.Args() + if len(args) == 0 { + return errors.New("at least one key=value argument is required") + } + + items, err := parseMetaDataBatchArgs(args) + if err != nil { + return err + } + + needles, _, err := redact.NeedlesFromEnv(cfg.RedactedVars) + if err != nil { + return err + } + + for i := range items { + if redactedValue := redact.String(items[i].Value, needles); redactedValue != items[i].Value { + l.Warn("Meta-data value for key %q contained one or more secrets from environment variables that have been redacted. If this is deliberate, pass --redacted-vars='' or a list of patterns that does not match the variable containing the secret", items[i].Key) + items[i].Value = redactedValue + } + } + + return setMetaDataBatch(ctx, cfg, l, items) + }, +} + +func parseMetaDataBatchArgs(args []string) ([]api.MetaData, error) { + items := make([]api.MetaData, 0, len(args)) + for _, arg := range args { + key, value, ok := strings.Cut(arg, "=") + if !ok { + return nil, fmt.Errorf("invalid argument %q: must be in key=value format", arg) + } + if strings.TrimSpace(key) == "" { + return nil, fmt.Errorf("invalid argument %q: key cannot be empty, or composed of only whitespace characters", arg) + } + if strings.TrimSpace(value) == "" { + return nil, fmt.Errorf("invalid argument %q: value cannot be empty, or composed of only whitespace characters", arg) + } + items = append(items, api.MetaData{Key: key, Value: value}) + } + return items, nil +} + +func setMetaDataBatch(ctx context.Context, cfg MetaDataSetBatchConfig, l logger.Logger, items []api.MetaData) error { + client := api.NewClient(l, loadAPIClientConfig(cfg, "AgentAccessToken")) + + batch := &api.MetaDataBatch{Items: items} + + if err := roko.NewRetrier( + roko.WithMaxAttempts(10), + roko.WithStrategy(roko.ExponentialSubsecond(2*time.Second)), + ).DoWithContext(ctx, func(r *roko.Retrier) error { + resp, err := client.SetMetaDataBatch(ctx, cfg.Job, batch) + if resp != nil && (resp.StatusCode == 401 || resp.StatusCode == 404) { + r.Break() + } + if err != nil { + l.Warn("%s (%s)", err, r) + return err + } + return nil + }); err != nil { + return fmt.Errorf("failed to set meta-data batch: %w", err) + } + + return nil +} diff --git a/clicommand/meta_data_set_batch_test.go b/clicommand/meta_data_set_batch_test.go new file mode 100644 index 0000000000..625a71e30e --- /dev/null +++ b/clicommand/meta_data_set_batch_test.go @@ -0,0 +1,223 @@ +package clicommand + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/buildkite/agent/v3/api" + "github.com/buildkite/agent/v3/logger" + "github.com/stretchr/testify/assert" +) + +func TestParseMetaDataBatchArgs(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + args []string + want []api.MetaData + wantErr string + }{ + { + name: "single pair", + args: []string{"foo=bar"}, + want: []api.MetaData{{Key: "foo", Value: "bar"}}, + }, + { + name: "multiple pairs", + args: []string{"a=1", "b=2", "c=3"}, + want: []api.MetaData{ + {Key: "a", Value: "1"}, + {Key: "b", Value: "2"}, + {Key: "c", Value: "3"}, + }, + }, + { + name: "value containing equals sign", + args: []string{"key=val=ue"}, + want: []api.MetaData{{Key: "key", Value: "val=ue"}}, + }, + { + name: "missing equals sign", + args: []string{"foobar"}, + wantErr: `invalid argument "foobar": must be in key=value format`, + }, + { + name: "empty key", + args: []string{"=bar"}, + wantErr: `invalid argument "=bar": key cannot be empty`, + }, + { + name: "whitespace-only key", + args: []string{" =bar"}, + wantErr: `invalid argument " =bar": key cannot be empty`, + }, + { + name: "empty value", + args: []string{"foo="}, + wantErr: `invalid argument "foo=": value cannot be empty`, + }, + { + name: "whitespace-only value", + args: []string{"foo= "}, + wantErr: `invalid argument "foo= ": value cannot be empty`, + }, + { + name: "error on first invalid stops parsing", + args: []string{"a=1", "bad", "c=3"}, + wantErr: `invalid argument "bad": must be in key=value format`, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got, err := parseMetaDataBatchArgs(tc.args) + if tc.wantErr != "" { + assert.ErrorContains(t, err, tc.wantErr) + return + } + assert.NoError(t, err) + assert.Equal(t, tc.want, got) + }) + } +} + +func TestSetMetaDataBatch(t *testing.T) { + t.Parallel() + + t.Run("success", func(t *testing.T) { + t.Parallel() + + var receivedBatch api.MetaDataBatch + server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + assert.Equal(t, "POST", req.Method) + assert.Equal(t, "/jobs/jobid/data/set-batch", req.URL.Path) + assert.NoError(t, json.NewDecoder(req.Body).Decode(&receivedBatch)) + rw.WriteHeader(http.StatusNoContent) + })) + defer server.Close() + + cfg := MetaDataSetBatchConfig{ + Job: "jobid", + APIConfig: APIConfig{ + AgentAccessToken: "agentaccesstoken", + Endpoint: server.URL, + }, + } + + items := []api.MetaData{ + {Key: "foo", Value: "bar"}, + {Key: "baz", Value: "qux"}, + } + + l := logger.NewBuffer() + err := setMetaDataBatch(context.Background(), cfg, l, items) + assert.NoError(t, err) + assert.Equal(t, items, receivedBatch.Items) + }) + + t.Run("server error", func(t *testing.T) { + t.Parallel() + + server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + rw.WriteHeader(http.StatusInternalServerError) + })) + defer server.Close() + + cfg := MetaDataSetBatchConfig{ + Job: "jobid", + APIConfig: APIConfig{ + AgentAccessToken: "agentaccesstoken", + Endpoint: server.URL, + }, + } + + items := []api.MetaData{{Key: "a", Value: "1"}} + + l := logger.NewBuffer() + err := setMetaDataBatch(context.Background(), cfg, l, items) + assert.ErrorContains(t, err, "failed to set meta-data batch") + }) + + t.Run("401 does not retry", func(t *testing.T) { + t.Parallel() + + callCount := 0 + server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + callCount++ + rw.WriteHeader(http.StatusUnauthorized) + })) + defer server.Close() + + cfg := MetaDataSetBatchConfig{ + Job: "jobid", + APIConfig: APIConfig{ + AgentAccessToken: "agentaccesstoken", + Endpoint: server.URL, + }, + } + + items := []api.MetaData{{Key: "a", Value: "1"}} + + l := logger.NewBuffer() + err := setMetaDataBatch(context.Background(), cfg, l, items) + assert.Error(t, err) + assert.Equal(t, 1, callCount) + }) + + t.Run("404 does not retry", func(t *testing.T) { + t.Parallel() + + callCount := 0 + server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + callCount++ + rw.WriteHeader(http.StatusNotFound) + })) + defer server.Close() + + cfg := MetaDataSetBatchConfig{ + Job: "jobid", + APIConfig: APIConfig{ + AgentAccessToken: "agentaccesstoken", + Endpoint: server.URL, + }, + } + + items := []api.MetaData{{Key: "a", Value: "1"}} + + l := logger.NewBuffer() + err := setMetaDataBatch(context.Background(), cfg, l, items) + assert.Error(t, err) + assert.Equal(t, 1, callCount) + }) + + t.Run("validation error from server", func(t *testing.T) { + t.Parallel() + + server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + rw.Header().Set("Content-Type", "application/json") + rw.WriteHeader(http.StatusUnprocessableEntity) + rw.Write([]byte(`{"message": "key cannot be blank"}`)) + })) + defer server.Close() + + cfg := MetaDataSetBatchConfig{ + Job: "jobid", + APIConfig: APIConfig{ + AgentAccessToken: "agentaccesstoken", + Endpoint: server.URL, + }, + } + + items := []api.MetaData{{Key: "a", Value: "1"}} + + l := logger.NewBuffer() + err := setMetaDataBatch(context.Background(), cfg, l, items) + assert.Error(t, err) + }) +}