From 17ddc301a82277ab2bf815d6c7157054a69ef2ff Mon Sep 17 00:00:00 2001 From: Haotian Date: Fri, 26 Aug 2022 13:37:26 -0700 Subject: [PATCH] feat(cli): add/set/remove worker tags (#2266) * adds CLI commands for add/set/removing worker tags, and respective tests * adds StringSliceMapVar Flag to convert a string from the CLI to a map[string][]string value --- api/workers/worker.gen.go | 206 ++++++++++++++++++ internal/api/genapi/input.go | 8 +- internal/cmd/base/base.go | 1 + internal/cmd/base/flags.go | 115 ++++++++++ internal/cmd/base/flags_test.go | 151 +++++++++++++ internal/cmd/commands.go | 18 ++ internal/cmd/commands/workerscmd/funcs.go | 145 +++++++++++- .../cmd/commands/workerscmd/workers.gen.go | 24 ++ internal/cmd/config/config.go | 6 + internal/cmd/config/config_test.go | 22 ++ internal/cmd/gencli/input.go | 2 +- internal/tests/api/workers/worker_test.go | 131 +++++++++++ 12 files changed, 825 insertions(+), 4 deletions(-) create mode 100644 internal/cmd/base/flags_test.go create mode 100644 internal/tests/api/workers/worker_test.go diff --git a/api/workers/worker.gen.go b/api/workers/worker.gen.go index 042234893..898410b64 100644 --- a/api/workers/worker.gen.go +++ b/api/workers/worker.gen.go @@ -329,3 +329,209 @@ func (c *Client) List(ctx context.Context, scopeId string, opt ...Option) (*Work target.response = resp return target, nil } + +func (c *Client) AddWorkerTags(ctx context.Context, id string, version uint32, apiTags map[string][]string, opt ...Option) (*WorkerUpdateResult, error) { + if id == "" { + return nil, fmt.Errorf("empty id value passed into AddWorkerTags request") + } + + if len(apiTags) == 0 { + return nil, errors.New("empty apiTags passed into AddWorkerTags request") + } + + if c.client == nil { + return nil, errors.New("nil client") + } + + opts, apiOpts := getOpts(opt...) + + if version == 0 { + if !opts.withAutomaticVersioning { + return nil, errors.New("zero version number passed into AddWorkerTags request") + } + existingTarget, existingErr := c.Read(ctx, id, append([]Option{WithSkipCurlOutput(true)}, opt...)...) + if existingErr != nil { + if api.AsServerError(existingErr) != nil { + return nil, fmt.Errorf("error from controller when performing initial check-and-set read: %w", existingErr) + } + return nil, fmt.Errorf("error performing initial check-and-set read: %w", existingErr) + } + if existingTarget == nil { + return nil, errors.New("nil resource response found when performing initial check-and-set read") + } + if existingTarget.Item == nil { + return nil, errors.New("nil resource found when performing initial check-and-set read") + } + version = existingTarget.Item.Version + } + + opts.postMap["version"] = version + + opts.postMap["api_tags"] = apiTags + + req, err := c.client.NewRequest(ctx, "POST", fmt.Sprintf("workers/%s:add-worker-tags", url.PathEscape(id)), opts.postMap, apiOpts...) + if err != nil { + return nil, fmt.Errorf("error creating AddWorkerTags request: %w", err) + } + + if len(opts.queryMap) > 0 { + q := url.Values{} + for k, v := range opts.queryMap { + q.Add(k, v) + } + req.URL.RawQuery = q.Encode() + } + + resp, err := c.client.Do(req) + if err != nil { + return nil, fmt.Errorf("error performing client request during AddWorkerTags call: %w", err) + } + + target := new(WorkerUpdateResult) + target.Item = new(Worker) + apiErr, err := resp.Decode(target.Item) + if err != nil { + return nil, fmt.Errorf("error decoding AddWorkerTags response: %w", err) + } + if apiErr != nil { + return nil, apiErr + } + target.response = resp + return target, nil +} + +func (c *Client) SetWorkerTags(ctx context.Context, id string, version uint32, apiTags map[string][]string, opt ...Option) (*WorkerUpdateResult, error) { + if id == "" { + return nil, fmt.Errorf("empty id value passed into SetWorkerTags request") + } + + if c.client == nil { + return nil, errors.New("nil client") + } + + opts, apiOpts := getOpts(opt...) + + if version == 0 { + if !opts.withAutomaticVersioning { + return nil, errors.New("zero version number passed into SetWorkerTags request") + } + existingTarget, existingErr := c.Read(ctx, id, append([]Option{WithSkipCurlOutput(true)}, opt...)...) + if existingErr != nil { + if api.AsServerError(existingErr) != nil { + return nil, fmt.Errorf("error from controller when performing initial check-and-set read: %w", existingErr) + } + return nil, fmt.Errorf("error performing initial check-and-set read: %w", existingErr) + } + if existingTarget == nil { + return nil, errors.New("nil resource response found when performing initial check-and-set read") + } + if existingTarget.Item == nil { + return nil, errors.New("nil resource found when performing initial check-and-set read") + } + version = existingTarget.Item.Version + } + + opts.postMap["version"] = version + + opts.postMap["api_tags"] = apiTags + + req, err := c.client.NewRequest(ctx, "POST", fmt.Sprintf("workers/%s:set-worker-tags", url.PathEscape(id)), opts.postMap, apiOpts...) + if err != nil { + return nil, fmt.Errorf("error creating SetWorkerTags request: %w", err) + } + + if len(opts.queryMap) > 0 { + q := url.Values{} + for k, v := range opts.queryMap { + q.Add(k, v) + } + req.URL.RawQuery = q.Encode() + } + + resp, err := c.client.Do(req) + if err != nil { + return nil, fmt.Errorf("error performing client request during SetWorkerTags call: %w", err) + } + + target := new(WorkerUpdateResult) + target.Item = new(Worker) + apiErr, err := resp.Decode(target.Item) + if err != nil { + return nil, fmt.Errorf("error decoding SetWorkerTags response: %w", err) + } + if apiErr != nil { + return nil, apiErr + } + target.response = resp + return target, nil +} + +func (c *Client) RemoveWorkerTags(ctx context.Context, id string, version uint32, apiTags map[string][]string, opt ...Option) (*WorkerUpdateResult, error) { + if id == "" { + return nil, fmt.Errorf("empty id value passed into RemoveWorkerTags request") + } + + if len(apiTags) == 0 { + return nil, errors.New("empty apiTags passed into RemoveWorkerTags request") + } + + if c.client == nil { + return nil, errors.New("nil client") + } + + opts, apiOpts := getOpts(opt...) + + if version == 0 { + if !opts.withAutomaticVersioning { + return nil, errors.New("zero version number passed into RemoveWorkerTags request") + } + existingTarget, existingErr := c.Read(ctx, id, append([]Option{WithSkipCurlOutput(true)}, opt...)...) + if existingErr != nil { + if api.AsServerError(existingErr) != nil { + return nil, fmt.Errorf("error from controller when performing initial check-and-set read: %w", existingErr) + } + return nil, fmt.Errorf("error performing initial check-and-set read: %w", existingErr) + } + if existingTarget == nil { + return nil, errors.New("nil resource response found when performing initial check-and-set read") + } + if existingTarget.Item == nil { + return nil, errors.New("nil resource found when performing initial check-and-set read") + } + version = existingTarget.Item.Version + } + + opts.postMap["version"] = version + + opts.postMap["api_tags"] = apiTags + + req, err := c.client.NewRequest(ctx, "POST", fmt.Sprintf("workers/%s:remove-worker-tags", url.PathEscape(id)), opts.postMap, apiOpts...) + if err != nil { + return nil, fmt.Errorf("error creating RemoveWorkerTags request: %w", err) + } + + if len(opts.queryMap) > 0 { + q := url.Values{} + for k, v := range opts.queryMap { + q.Add(k, v) + } + req.URL.RawQuery = q.Encode() + } + + resp, err := c.client.Do(req) + if err != nil { + return nil, fmt.Errorf("error performing client request during RemoveWorkerTags call: %w", err) + } + + target := new(WorkerUpdateResult) + target.Item = new(Worker) + apiErr, err := resp.Decode(target.Item) + if err != nil { + return nil, fmt.Errorf("error decoding RemoveWorkerTags response: %w", err) + } + if apiErr != nil { + return nil, apiErr + } + target.response = resp + return target, nil +} diff --git a/internal/api/genapi/input.go b/internal/api/genapi/input.go index 6c08b223f..b7c402854 100644 --- a/internal/api/genapi/input.go +++ b/internal/api/genapi/input.go @@ -810,7 +810,13 @@ var inputStructs = []*structInfo{ PostType: "worker_generated_auth_token", }, }, - pluralResourceName: "workers", + pluralResourceName: "workers", + sliceSubtypes: map[string]sliceSubtypeInfo{ + "WorkerTags": { + SliceType: "map[string][]string", + VarName: "apiTags", + }, + }, createResponseTypes: true, recursiveListing: true, versionEnabled: true, diff --git a/internal/cmd/base/base.go b/internal/cmd/base/base.go index 98f5a8c7c..04a6b0eca 100644 --- a/internal/cmd/base/base.go +++ b/internal/cmd/base/base.go @@ -113,6 +113,7 @@ type Command struct { FlagVersion int FlagRecursive bool FlagFilter string + FlagTags map[string][]string // Attribute values FlagAttributes string diff --git a/internal/cmd/base/flags.go b/internal/cmd/base/flags.go index 34dfbc87c..9ee7dad52 100644 --- a/internal/cmd/base/flags.go +++ b/internal/cmd/base/flags.go @@ -716,6 +716,121 @@ func mapToKV(m map[string]string) string { return strings.Join(list, ",") } +// StringSliceMapVar maps a key string to a slice of string values. +// This is useful for cases such as modifying Worker tags, which can have +// multiple values associated per key. +// Setting NullCheck to return true enables the string input value "null" +// to be accepted without throwing an error. This is useful for add/set/remove +// patterns where we want to enable "set null". +type StringSliceMapVar struct { + Name string + Aliases []string + Usage string + Default map[string][]string + Hidden bool + Target *map[string][]string + NullCheck func() bool + Completion complete.Predictor +} + +func (f *FlagSet) StringSliceMapVar(i *StringSliceMapVar) { + f.VarFlag(&VarFlag{ + Name: i.Name, + Aliases: i.Aliases, + Usage: i.Usage, + Value: newStringSliceMapValue(i.Default, i.Target, i.NullCheck, i.Hidden), + Completion: i.Completion, + }) +} + +type stringSliceMapValue struct { + hidden bool + target *map[string][]string + nullCheck func() bool + isNull bool +} + +func newStringSliceMapValue(def map[string][]string, target *map[string][]string, nullCheck func() bool, hidden bool) *stringSliceMapValue { + *target = def + return &stringSliceMapValue{ + hidden: hidden, + target: target, + nullCheck: nullCheck, + isNull: false, + } +} + +func (s *stringSliceMapValue) Set(val string) error { + kv := strings.TrimSpace(val) + + // Don't enable this behavior if nullCheck is not defined. + if s.nullCheck != nil { + if kv == "null" { + if !s.nullCheck() { + return fmt.Errorf(`"null" is not an allowed value`) + } + // Ensure "null" cannot be entered as a consecutive entry in a chain of valid entries + if s.isNull == true || len(*s.target) > 0 { + return fmt.Errorf(`"null" cannot be combined with other values`) + } + // Set target to nil and return + if kv == "null" { + *s.target = map[string][]string{"null": nil} + s.isNull = true + return nil + } + } else if s.isNull == true { + // Ensure consecutive "non-null" entries cannot be entered after a "null" + return fmt.Errorf(`"null" cannot be combined with other values`) + } + } + // Return a better error message if "null" is not enabled + if kv == "null" { + return fmt.Errorf(`"null" is not an allowed value`) + } + + if *s.target == nil { + *s.target = make(map[string][]string) + } + split := strings.SplitN(kv, "=", 2) + if len(split) != 2 { + return fmt.Errorf("missing = in KV pair: %q", val) + } + + // trim space, check proto, and assign to map + key := strings.TrimSpace(split[0]) + if !protoIdentifierRegex.Match([]byte(key)) { + return fmt.Errorf("key %q is invalid", key) + } + + vals := strings.Split(split[1], ",") + for i, v := range vals { + vals[i] = strings.TrimSpace(v) + if !protoIdentifierRegex.Match([]byte(vals[i])) { + return fmt.Errorf("value %q is invalid", vals[i]) + } + } + for _, v := range vals { + (*s.target)[key] = append((*s.target)[key], v) + } + + return nil +} + +func sliceMapToKV(m map[string][]string) string { + list := make([]string, 0, len(m)) + for k, vals := range m { + vv := strings.Join(vals, ",") + list = append(list, k+"="+vv) + } + return strings.Join(list, ", ") +} + +func (c *stringSliceMapValue) Get() interface{} { return *c.target } +func (c *stringSliceMapValue) String() string { return sliceMapToKV(*c.target) } +func (c *stringSliceMapValue) Example() string { return "key1=val-a, key2=val-b,val-c" } +func (c *stringSliceMapValue) Hidden() bool { return c.hidden } + // -- VarFlag type VarFlag struct { Name string diff --git a/internal/cmd/base/flags_test.go b/internal/cmd/base/flags_test.go new file mode 100644 index 000000000..ce0cd9f6b --- /dev/null +++ b/internal/cmd/base/flags_test.go @@ -0,0 +1,151 @@ +package base + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestFlagSet_StringSliceMapVar(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + setString string + wantErrContains string + wantTarget map[string][]string + }{ + { + name: "key-value", + setString: "key=value", + wantTarget: map[string][]string{"key": {"value"}}, + }, + { + name: "many-values", + setString: "key=v1,v2,v3,v4,v5", + wantTarget: map[string][]string{"key": {"v1", "v2", "v3", "v4", "v5"}}, + }, + { + // this case passes here and errors at worker client + name: "duplicate-values", + setString: "k1=v1,v1", + wantTarget: map[string][]string{"k1": {"v1", "v1"}}, + }, + { + name: "no-deliminator", + setString: "key", + wantErrContains: "missing = in KV pair", + }, + { + name: "multiple-keys", + setString: "k1=v1, k2=v2", + wantErrContains: "value \"k2=v2\" is invalid", + }, + { + name: "too-many-commas", + setString: "k1=v1,,v3?,,,,,v8", + wantErrContains: "value \"\" is invalid", + }, + { + name: "illegal-characters-key", + setString: "!@#$=%^&*()_", + wantErrContains: "key \"!@#$\" is invalid", + }, + { + name: "illegal-characters-value", + setString: "k=%^&*()_", + wantErrContains: "value \"%^&*()_\" is invalid", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + + target := make(map[string][]string, 1) + sliceMapVal := &stringSliceMapValue{ + hidden: false, + target: &target, + } + + err := sliceMapVal.Set(tt.setString) + if tt.wantErrContains != "" { + assert.Error(err) + assert.Contains(err.Error(), tt.wantErrContains) + return + } + require.NoError(err) + assert.Equal(tt.wantTarget, *sliceMapVal.target) + }) + } +} + +func TestFlagSet_StringSliceMapVar_NullCheck(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + setStrings []string + wantErrContains string + wantTarget map[string][]string + }{ + { + name: "null", + setStrings: []string{"null"}, + wantTarget: map[string][]string{"null": nil}, + // this target is then converted to a nil value in workerscmd/func.go + }, + { + name: "glass-half-null", + setStrings: []string{"milk=half", "null"}, + wantErrContains: `"null" cannot be combined with other values`, + }, + { + name: "invalid-entry-before", + setStrings: []string{"the-milk-is-bad", "null"}, + wantErrContains: `missing = in KV pair: "the-milk-is-bad"`, + }, + { + name: "invalid-entry-after", + setStrings: []string{"null", "the-milk-is-still-bad"}, + wantErrContains: `"null" cannot be combined with other values`, + }, + { + name: "no-error", + setStrings: []string{"k1=v1,v2,v3", "k2=v4,v5,v6"}, + wantTarget: map[string][]string{"k1": {"v1", "v2", "v3"}, "k2": {"v4", "v5", "v6"}}, + }, + { + name: "too-null", + setStrings: []string{"null", "null", "null"}, + wantErrContains: `"null" cannot be combined with other values`, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + + target := make(map[string][]string, 1) + sliceMapVal := &stringSliceMapValue{ + hidden: false, + target: &target, + nullCheck: func() bool { return true }, + } + var err error + + for _, s := range tt.setStrings { + err = sliceMapVal.Set(s) + if err != nil { + break + } + } + if tt.wantErrContains != "" { + assert.Error(err) + assert.Contains(err.Error(), tt.wantErrContains) + return + } + require.NoError(err) + assert.Equal(tt.wantTarget, *sliceMapVal.target) + }) + } +} diff --git a/internal/cmd/commands.go b/internal/cmd/commands.go index 77379d410..a76d021fa 100644 --- a/internal/cmd/commands.go +++ b/internal/cmd/commands.go @@ -1138,6 +1138,24 @@ func initCommands(ui, serverCmdUi cli.Ui, runOpts *RunOptions) { Func: "list", }, nil }, + "workers add-worker-tags": func() (cli.Command, error) { + return &workerscmd.Command{ + Command: base.NewCommand(ui), + Func: "add-worker-tags", + }, nil + }, + "workers set-worker-tags": func() (cli.Command, error) { + return &workerscmd.Command{ + Command: base.NewCommand(ui), + Func: "set-worker-tags", + }, nil + }, + "workers remove-worker-tags": func() (cli.Command, error) { + return &workerscmd.Command{ + Command: base.NewCommand(ui), + Func: "remove-worker-tags", + }, nil + }, } for _, fn := range extraCommandsFuncs { diff --git a/internal/cmd/commands/workerscmd/funcs.go b/internal/cmd/commands/workerscmd/funcs.go index 550d007b9..328484ddc 100644 --- a/internal/cmd/commands/workerscmd/funcs.go +++ b/internal/cmd/commands/workerscmd/funcs.go @@ -2,6 +2,7 @@ package workerscmd import ( "fmt" + "strings" "time" "github.com/hashicorp/boundary/api" @@ -10,6 +11,35 @@ import ( "github.com/hashicorp/boundary/internal/cmd/base" ) +func init() { + extraActionsFlagsMapFunc = extraActionsFlagsMapFuncImpl + extraSynopsisFunc = extraSynopsisFuncImpl + extraFlagsFunc = extraFlagsFuncImpl + extraFlagsHandlingFunc = extraFlagsHandlingFuncImpl + executeExtraActions = executeExtraActionsImpl +} + +func extraActionsFlagsMapFuncImpl() map[string][]string { + return map[string][]string{ + "add-worker-tags": {"id", "tag", "version"}, + "set-worker-tags": {"id", "tag", "version"}, + "remove-worker-tags": {"id", "tag", "version"}, + } +} + +func extraSynopsisFuncImpl(c *Command) string { + switch c.Func { + case "add-worker-tags": + return "Add api tags to the specified worker" + case "set-worker-tags": + return "Set api tags for the specified worker" + case "remove-worker-tags": + return "Remove api tags from the specified worker" + default: + return "" + } +} + func (c *Command) extraHelpFunc(helpMap map[string]func() string) string { var helpStr string switch c.Func { @@ -25,12 +55,113 @@ func (c *Command) extraHelpFunc(helpMap map[string]func() string) string { "", " Please see the workers subcommand help for detailed usage information.", }) + case "add-worker-tags": + helpStr = base.WrapForHelpText([]string{ + "Usage: boundary workers add-worker-tags [sub command] [args]", + "", + " This command allows adding api tags to worker resources. Example:", + "", + " Add a set of api tags to a specified worker:", + "", + ` & boundary workers add-worker-tags -id w_1234567890 -tag "key1=val-a" -tag "key2=val-b,val-c"`, + "", + "", + }) + case "set-worker-tags": + helpStr = base.WrapForHelpText([]string{ + "Usage: boundary workers set-worker-tags [sub command] [args]", + "", + " This command allows setting api tags for worker resources. Example:", + "", + " Set api tags for a specified worker:", + "", + ` & boundary workers set-worker-tags -id w_1234567890 -tag "key1=val-a" -tag "key2=val-b,val-c"`, + "", + "", + }) + case "remove-worker-tags": + helpStr = base.WrapForHelpText([]string{ + "Usage: boundary workers remove-worker-tags [sub command] [args]", + "", + " This command allows removing api tags from worker resources. Example:", + "", + " Remove a set of api tags to a specified worker:", + "", + ` & boundary workers remove-worker-tags -id w_1234567890 -tag "key1=val-a" -tag "key2=val-b,val-c"`, + "", + "", + }) default: helpStr = helpMap[c.Func]() } return helpStr + c.Flags().Help() } +func extraFlagsFuncImpl(c *Command, _ *base.FlagSets, f *base.FlagSet) { + for _, name := range flagsMap[c.Func] { + switch name { + case "tag": + var nullCheckFn func() bool = nil + switch { + case strings.ToLower(c.Func[:3]) == "set": + nullCheckFn = func() bool { return true } + default: + } + f.StringSliceMapVar(&base.StringSliceMapVar{ + Name: "tag", + Target: &c.FlagTags, + NullCheck: nullCheckFn, + Usage: "The api tag resources to add, remove, or set.", + }) + } + } +} + +func extraFlagsHandlingFuncImpl(c *Command, _ *base.FlagSets, opts *[]workers.Option) bool { + switch c.Func { + case "add-worker-tags", "remove-worker-tags": + if len(c.FlagTags) == 0 { + c.UI.Error("No tags supplied via -tag") + return false + } + case "set-worker-tags": + switch len(c.FlagTags) { + case 0: + c.UI.Error("No tags supplied via -tag") + return false + case 1: + if c.FlagTags["null"] == nil { + c.FlagTags = nil + } + } + } + return true +} + +func executeExtraActionsImpl(c *Command, inResp *api.Response, inItem *workers.Worker, inItems []*workers.Worker, inErr error, workerClient *workers.Client, version uint32, opts []workers.Option) (*api.Response, *workers.Worker, []*workers.Worker, error) { + switch c.Func { + case "add-worker-tags": + result, err := workerClient.AddWorkerTags(c.Context, c.FlagId, version, c.FlagTags, opts...) + if err != nil { + return nil, nil, nil, err + } + return result.GetResponse(), result.GetItem(), nil, err + case "set-worker-tags": + result, err := workerClient.SetWorkerTags(c.Context, c.FlagId, version, c.FlagTags, opts...) + if err != nil { + return nil, nil, nil, err + } + return result.GetResponse(), result.GetItem(), nil, err + case "remove-worker-tags": + result, err := workerClient.RemoveWorkerTags(c.Context, c.FlagId, version, c.FlagTags, opts...) + if err != nil { + return nil, nil, nil, err + } + return result.GetResponse(), result.GetItem(), nil, err + } + return inResp, inItem, inItems, inErr +} + func (c *Command) printListTable(items []*workers.Worker) string { if len(items) == 0 { return "No workers found" @@ -152,7 +283,7 @@ func printItemTable(item *workers.Worker, resp *api.Response) string { ) } - if len(item.CanonicalTags) > 0 || len(item.ConfigTags) > 0 { + if len(item.CanonicalTags) > 0 || len(item.ApiTags) > 0 || len(item.ConfigTags) > 0 { ret = append(ret, "", " Tags:", @@ -163,7 +294,17 @@ func printItemTable(item *workers.Worker, resp *api.Response) string { tagMap[k] = v } ret = append(ret, - " Worker Configuration:", + " Configuration:", + base.WrapMap(6, 2, tagMap), + ) + } + if len(item.ApiTags) > 0 { + tagMap := make(map[string]any, len(item.ApiTags)) + for k, v := range item.ApiTags { + tagMap[k] = v + } + ret = append(ret, + " Api:", base.WrapMap(6, 2, tagMap), ) } diff --git a/internal/cmd/commands/workerscmd/workers.gen.go b/internal/cmd/commands/workerscmd/workers.gen.go index 738e4535b..49707659b 100644 --- a/internal/cmd/commands/workerscmd/workers.gen.go +++ b/internal/cmd/commands/workerscmd/workers.gen.go @@ -208,6 +208,30 @@ func (c *Command) Run(args []string) int { version = uint32(c.FlagVersion) } + case "add-worker-tags": + switch c.FlagVersion { + case 0: + opts = append(opts, workers.WithAutomaticVersioning(true)) + default: + version = uint32(c.FlagVersion) + } + + case "set-worker-tags": + switch c.FlagVersion { + case 0: + opts = append(opts, workers.WithAutomaticVersioning(true)) + default: + version = uint32(c.FlagVersion) + } + + case "remove-worker-tags": + switch c.FlagVersion { + case 0: + opts = append(opts, workers.WithAutomaticVersioning(true)) + default: + version = uint32(c.FlagVersion) + } + } if ok := extraFlagsHandlingFunc(c, f, &opts); !ok { diff --git a/internal/cmd/config/config.go b/internal/cmd/config/config.go index 1bed08111..0a69215a5 100644 --- a/internal/cmd/config/config.go +++ b/internal/cmd/config/config.go @@ -546,6 +546,9 @@ func Parse(d string) (*Config, error) { if !strutil.Printable(k) { return nil, fmt.Errorf("Tag key %q contains non-printable characters", k) } + if strings.Contains(k, ",") { + return nil, fmt.Errorf("Tag key %q cannot contain commas", k) + } for _, val := range v { if val != strings.ToLower(val) { return nil, fmt.Errorf("Tag value %q for tag key %q is not all lower-case letters", val, k) @@ -553,6 +556,9 @@ func Parse(d string) (*Config, error) { if !strutil.Printable(k) { return nil, fmt.Errorf("Tag value %q for tag key %q contains non-printable characters", v, k) } + if strings.Contains(val, ",") { + return nil, fmt.Errorf("Tag value %q for tag key %q cannot contain commas", val, k) + } } } diff --git a/internal/cmd/config/config_test.go b/internal/cmd/config/config_test.go index 1af326bd4..1f4ac86f1 100644 --- a/internal/cmd/config/config_test.go +++ b/internal/cmd/config/config_test.go @@ -666,6 +666,28 @@ func TestWorkerTags(t *testing.T) { }, expErr: false, }, + { + name: "comma in tag key string", + in: ` + worker { + tags { + "key,"= ["value"], + } + }`, + expErr: true, + expErrStr: `Tag key "key," cannot contain commas`, + }, + { + name: "comma in tag value string", + in: ` + worker { + tags { + "key"= ["va,lue","value2"], + } + }`, + expErr: true, + expErrStr: `Tag value "va,lue" for tag key "key" cannot contain commas`, + }, { name: "json tags - entire tags block", in: ` diff --git a/internal/cmd/gencli/input.go b/internal/cmd/gencli/input.go index 540ef6742..65b50489e 100644 --- a/internal/cmd/gencli/input.go +++ b/internal/cmd/gencli/input.go @@ -535,7 +535,7 @@ var inputStructs = map[string][]*cmdInfo{ Container: "Scope", HasName: true, HasDescription: true, - VersionedActions: []string{"update"}, + VersionedActions: []string{"update", "add-worker-tags", "set-worker-tags", "remove-worker-tags"}, }, { ResourceType: resource.Worker.String(), diff --git a/internal/tests/api/workers/worker_test.go b/internal/tests/api/workers/worker_test.go new file mode 100644 index 000000000..d2b20b6fd --- /dev/null +++ b/internal/tests/api/workers/worker_test.go @@ -0,0 +1,131 @@ +package workers_test + +import ( + "context" + "crypto/rand" + "testing" + + "github.com/hashicorp/boundary/api/workers" + "github.com/hashicorp/boundary/internal/daemon/controller" + "github.com/hashicorp/boundary/internal/db" + "github.com/hashicorp/boundary/internal/kms" + "github.com/hashicorp/boundary/internal/server" + "github.com/hashicorp/boundary/internal/types/scope" + "github.com/hashicorp/nodeenrollment/rotation" + "github.com/hashicorp/nodeenrollment/storage/file" + "github.com/hashicorp/nodeenrollment/types" + "github.com/mr-tron/base58" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/proto" +) + +func TestWorkerTagsASD(t *testing.T) { + ctx := context.Background() + assert, require := assert.New(t), require.New(t) + tc := controller.NewTestController(t, nil) + defer tc.Shutdown() + + client := tc.Client() + token := tc.Token() + client.SetToken(token.Token) + conn, _ := db.TestSetup(t, "postgres") + rw := db.New(conn) + wrapper := db.TestWrapper(t) + kmsCache := kms.TestKms(t, conn, wrapper) + + // Ensures the global scope contains a valid root key + err := kmsCache.CreateKeys(ctx, scope.Global.String(), kms.WithRandomReader(rand.Reader)) + require.NoError(err) + wrapper, err = kmsCache.GetWrapper(ctx, scope.Global.String(), kms.KeyPurposeDatabase) + require.NoError(err) + require.NotNil(wrapper) + + // Set up certs on the controller + rootStorage, err := server.NewRepositoryStorage(ctx, rw, rw, kmsCache) + require.NoError(err) + _, err = rotation.RotateRootCertificates(ctx, rootStorage) + require.NoError(err) + + // Emulate creating credentials on a worker + fileStorage, err := file.New(ctx) + require.NoError(err) + nodeCreds, err := types.NewNodeCredentials(ctx, fileStorage) + require.NoError(err) + fetchReq, err := nodeCreds.CreateFetchNodeCredentialsRequest(ctx) + require.NoError(err) + + // Create a workerAuth request to be passed to the workerClient + reqBytes, err := proto.Marshal(fetchReq) + require.NoError(err) + registrationReq := base58.FastBase58Encoding(reqBytes) + + workerClient := workers.NewClient(client) + + wcr, err := workerClient.CreateWorkerLed(tc.Context(), registrationReq, scope.Global.String()) + require.NoError(err) + + inputTags := map[string][]string{"key": {"value"}} + wcr, err = workerClient.AddWorkerTags(tc.Context(), wcr.Item.Id, wcr.Item.Version, inputTags) + require.NoError(err) + require.NotNil(wcr) + for k, v := range wcr.Item.ApiTags { + for expK, expV := range inputTags { + assert.Equal(k, expK) + assert.ElementsMatch(v, expV) + } + } + + inputTags = map[string][]string{"key2": {"value2", "value3"}} + wcr, err = workerClient.SetWorkerTags(tc.Context(), wcr.Item.Id, wcr.Item.Version, inputTags) + require.NoError(err) + require.NotNil(wcr) + for k, v := range wcr.Item.ApiTags { + for expK, expV := range inputTags { + assert.Equal(k, expK) + assert.ElementsMatch(v, expV) + } + } + + inputTags = map[string][]string{"key2": {"value3"}} + wcr, err = workerClient.RemoveWorkerTags(tc.Context(), wcr.Item.Id, wcr.Item.Version, inputTags) + require.NoError(err) + require.NotNil(wcr) + for k, v := range wcr.Item.ApiTags { + for expK := range inputTags { + assert.Equal(k, expK) + assert.ElementsMatch(v, []string{"value2"}) + } + } + + wcr, err = workerClient.SetWorkerTags(tc.Context(), wcr.Item.Id, wcr.Item.Version, nil) + require.NoError(err) + require.NotNil(wcr) + assert.Empty(wcr.Item.ApiTags) + + // Test adding the same tag twice + inputTags = map[string][]string{"keykey": {"valval"}} + wcr, err = workerClient.AddWorkerTags(tc.Context(), wcr.Item.Id, wcr.Item.Version, inputTags) + require.NoError(err) + require.NotNil(wcr) + for k, v := range wcr.Item.ApiTags { + for expK, expV := range inputTags { + assert.Equal(k, expK) + assert.ElementsMatch(v, expV) + } + } + inputTags = map[string][]string{"keykey": {"valval"}} + ewcr, err := workerClient.AddWorkerTags(tc.Context(), wcr.Item.Id, wcr.Item.Version, inputTags) + require.Error(err) + require.Nil(ewcr) + + inputTags = map[string][]string{"invalid_tag": nil} + ewcr, err = workerClient.SetWorkerTags(tc.Context(), wcr.Item.Id, wcr.Item.Version, inputTags) + require.Error(err) + require.Nil(ewcr) + + inputTags = map[string][]string{"non_extant": {"tag"}} + ewcr, err = workerClient.RemoveWorkerTags(tc.Context(), wcr.Item.Id, wcr.Item.Version, inputTags) + require.Error(err) + require.Nil(ewcr) +}