-
Notifications
You must be signed in to change notification settings - Fork 351
Add meta-data set-batch command
#3791
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 <key=value>... [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) | ||
| } | ||
|
Comment on lines
+98
to
+100
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we offer a way to "clear" a particular key? |
||
| 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() | ||
|
Comment on lines
+116
to
+117
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In Useful? React with 👍 / 👎.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 Valid observation, but this intentionally matches the existing |
||
| } | ||
| 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 | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we support JSON input (over stdin)?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe!
Do you think that would change the CLI design? 🤔
Would it look like one/some of these…
I mostly didn't add it because it contained more decisions 😅
If we think the current
argvapproach needs to change to make room for JSON/stdin input, I'll revise it. But if we think there's room at add it later, I'd rather defer it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(In my initial use-case, I have a bash script coordinating some build concerns, and it's rounding up lots of information and then setting ~8 meta-data key=values. I could push that into JSON structure, perhaps using
jqas a builder, but it's a much better fit to just build an array ofkey=valuestrings and put them on argv)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I foresee it being useful if something other than Bash is orchestrating. We have JSON input in some other commands. But I agree it can be done later.