From 33554627a9e1ba0eda159edf0711fdccc53b4de8 Mon Sep 17 00:00:00 2001 From: Grace Rehn Date: Fri, 3 Oct 2025 16:13:41 +1000 Subject: [PATCH 1/3] feat: Extend tag sets and add environment tags --- go.mod | 2 +- go.sum | 4 +- pkg/cmd/environment/create/create.go | 53 ++++++- pkg/cmd/environment/environment.go | 2 + pkg/cmd/environment/list/list.go | 14 +- pkg/cmd/environment/tag/tag.go | 137 +++++++++++++++++++ pkg/cmd/environment/tag/tag_opts.go | 116 ++++++++++++++++ pkg/cmd/tenant/create/create.go | 44 +++--- pkg/cmd/tenant/create/create_opts.go | 7 +- pkg/cmd/tenant/create/create_test.go | 66 +++++---- pkg/cmd/tenant/tag/tag.go | 44 +++--- pkg/cmd/tenant/tag/tag_opts.go | 7 +- pkg/question/selectors/tags.go | 181 ++++++++++++++++++++++++ pkg/question/selectors/tags_test.go | 197 +++++++++++++++++++++++++++ 14 files changed, 781 insertions(+), 93 deletions(-) create mode 100644 pkg/cmd/environment/tag/tag.go create mode 100644 pkg/cmd/environment/tag/tag_opts.go create mode 100644 pkg/question/selectors/tags.go create mode 100644 pkg/question/selectors/tags_test.go diff --git a/go.mod b/go.mod index b63f7d8f..0eedb856 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( github.com/AlecAivazis/survey/v2 v2.3.7 github.com/MakeNowJust/heredoc/v2 v2.0.1 github.com/OctopusDeploy/go-octodiff v1.0.0 - github.com/OctopusDeploy/go-octopusdeploy/v2 v2.82.0 + github.com/OctopusDeploy/go-octopusdeploy/v2 v2.84.1 github.com/bmatcuk/doublestar/v4 v4.4.0 github.com/briandowns/spinner v1.19.0 github.com/google/uuid v1.3.0 diff --git a/go.sum b/go.sum index 2f468c58..f7696fba 100644 --- a/go.sum +++ b/go.sum @@ -46,8 +46,8 @@ github.com/Netflix/go-expect v0.0.0-20220104043353-73e0943537d2 h1:+vx7roKuyA63n github.com/Netflix/go-expect v0.0.0-20220104043353-73e0943537d2/go.mod h1:HBCaDeC1lPdgDeDbhX8XFpy1jqjK0IBG8W5K+xYqA0w= github.com/OctopusDeploy/go-octodiff v1.0.0 h1:U+ORg6azniwwYo+O44giOw6TiD5USk8S4VDhOQ0Ven0= github.com/OctopusDeploy/go-octodiff v1.0.0/go.mod h1:Mze0+EkOWTgTmi8++fyUc6r0aLZT7qD9gX+31t8MmIU= -github.com/OctopusDeploy/go-octopusdeploy/v2 v2.82.0 h1:4Pc2W74VKp7Qm0uV0Dv99QKqRWg8WriVikdZPBpIZgY= -github.com/OctopusDeploy/go-octopusdeploy/v2 v2.82.0/go.mod h1:J1UdIilp41MRuFl+5xZm88ywFqJGYCCqxqod+/ZH8ko= +github.com/OctopusDeploy/go-octopusdeploy/v2 v2.84.1 h1:7BfRa64Jxb6XNqnMiAXMza5K9C4nt8fcYGFEVQ/MfuI= +github.com/OctopusDeploy/go-octopusdeploy/v2 v2.84.1/go.mod h1:J1UdIilp41MRuFl+5xZm88ywFqJGYCCqxqod+/ZH8ko= github.com/bmatcuk/doublestar/v4 v4.4.0 h1:LmAwNwhjEbYtyVLzjcP/XeVw4nhuScHGkF/XWXnvIic= github.com/bmatcuk/doublestar/v4 v4.4.0/go.mod h1:xBQ8jztBU6kakFMg+8WGxn0c6z1fTSPVIjEY1Wr7jzc= github.com/briandowns/spinner v1.19.0 h1:s8aq38H+Qju89yhp89b4iIiMzMm8YN3p6vGpwyh/a8E= diff --git a/pkg/cmd/environment/create/create.go b/pkg/cmd/environment/create/create.go index e3b70d30..fe208eba 100644 --- a/pkg/cmd/environment/create/create.go +++ b/pkg/cmd/environment/create/create.go @@ -9,8 +9,11 @@ import ( "github.com/OctopusDeploy/cli/pkg/factory" "github.com/OctopusDeploy/cli/pkg/output" "github.com/OctopusDeploy/cli/pkg/question" + "github.com/OctopusDeploy/cli/pkg/question/selectors" "github.com/OctopusDeploy/cli/pkg/util/flag" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/client" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/environments" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/tagsets" "github.com/spf13/cobra" ) @@ -19,6 +22,7 @@ const ( FlagDescription = "description" FlagUseGuidedFailure = "use-guided-failure" FlagDynamicInfrastructure = "allow-dynamic-infrastructure" + FlagTag = "tag" ) type CreateFlags struct { @@ -26,6 +30,7 @@ type CreateFlags struct { Description *flag.Flag[string] GuidedFailureMode *flag.Flag[bool] DynamicInfrastructure *flag.Flag[bool] + Tag *flag.Flag[[]string] } func NewCreateFlags() *CreateFlags { @@ -34,18 +39,23 @@ func NewCreateFlags() *CreateFlags { Description: flag.New[string](FlagDescription, false), GuidedFailureMode: flag.New[bool](FlagUseGuidedFailure, false), DynamicInfrastructure: flag.New[bool](FlagDynamicInfrastructure, false), + Tag: flag.New[[]string](FlagTag, false), } } +type GetAllTagSetsCallback func() ([]*tagsets.TagSet, error) + type CreateOptions struct { *CreateFlags *cmd.Dependencies + GetAllTagsCallback GetAllTagSetsCallback } func NewCreateOptions(createFlags *CreateFlags, dependencies *cmd.Dependencies) *CreateOptions { return &CreateOptions{ - CreateFlags: createFlags, - Dependencies: dependencies, + CreateFlags: createFlags, + Dependencies: dependencies, + GetAllTagsCallback: getAllTagSetsCallback(dependencies.Client), } } @@ -70,6 +80,7 @@ func NewCmdCreate(f factory.Factory) *cobra.Command { flags.StringVarP(&createFlags.Description.Value, createFlags.Description.Name, "d", "", "Description of the environment") flags.BoolVar(&createFlags.GuidedFailureMode.Value, createFlags.GuidedFailureMode.Name, false, "Use guided failure mode by default") flags.BoolVar(&createFlags.DynamicInfrastructure.Value, createFlags.DynamicInfrastructure.Name, false, "Allow dynamic infrastructure") + flags.StringArrayVarP(&createFlags.Tag.Value, createFlags.Tag.Name, "t", []string{}, "Tag to apply to environment, must use canonical name: /") return cmd } @@ -80,12 +91,24 @@ func createRun(opts *CreateOptions) error { if err != nil { return err } + } else { + // Validate tags when running with --no-prompt + if len(opts.Tag.Value) > 0 { + tagSets, err := opts.GetAllTagsCallback() + if err != nil { + return err + } + if err := selectors.ValidateTags(opts.Tag.Value, tagSets); err != nil { + return err + } + } } env := environments.NewEnvironment(opts.Name.Value) env.Description = opts.Description.Value env.AllowDynamicInfrastructure = opts.DynamicInfrastructure.Value env.UseGuidedFailure = opts.GuidedFailureMode.Value + env.EnvironmentTags = opts.Tag.Value createEnv, err := opts.Client.Environments.Add(env) if err != nil { @@ -101,7 +124,7 @@ func createRun(opts *CreateOptions) error { fmt.Fprintf(opts.Out, "View this environment on Octopus Deploy: %s\n", link) if !opts.NoPrompt { - autoCmd := flag.GenerateAutomationCmd(opts.CmdPath, opts.Name, opts.Description, opts.GuidedFailureMode, opts.DynamicInfrastructure) + autoCmd := flag.GenerateAutomationCmd(opts.CmdPath, opts.Name, opts.Description, opts.GuidedFailureMode, opts.DynamicInfrastructure, opts.Tag) fmt.Fprintf(opts.Out, "%s\n", autoCmd) } @@ -122,6 +145,17 @@ func PromptMissing(opts *CreateOptions) error { _, err = promptBool(opts, &opts.GuidedFailureMode.Value, false, "Use guided failure", "If guided failure is enabled for an environment, Octopus Deploy will prompt for user intervention if a deployment fails in the environment.") _, err = promptBool(opts, &opts.DynamicInfrastructure.Value, false, "Allow dynamic infrastructure", "If dynamic infrastructure is enabled for an environment, deployments to this environment are allowed to create infrastructure, such as targets and accounts.") + tagSets, err := opts.GetAllTagsCallback() + if err != nil { + return err + } + + tags, err := selectors.Tags(opts.Ask, []string{}, opts.Tag.Value, tagSets) + if err != nil { + return err + } + opts.Tag.Value = tags + return nil } @@ -136,3 +170,16 @@ func promptBool(opts *CreateOptions, value *bool, defaultValue bool, message str }, value) return *value, err } + +func getAllTagSetsCallback(client *client.Client) GetAllTagSetsCallback { + return func() ([]*tagsets.TagSet, error) { + query := tagsets.TagSetsQuery{ + Scopes: []string{string(tagsets.TagSetScopeEnvironment)}, + } + result, err := tagsets.Get(client, client.GetSpaceID(), query) + if err != nil { + return nil, err + } + return result.Items, nil + } +} diff --git a/pkg/cmd/environment/environment.go b/pkg/cmd/environment/environment.go index 8c26e851..9978ef4f 100644 --- a/pkg/cmd/environment/environment.go +++ b/pkg/cmd/environment/environment.go @@ -5,6 +5,7 @@ import ( cmdCreate "github.com/OctopusDeploy/cli/pkg/cmd/environment/create" cmdDelete "github.com/OctopusDeploy/cli/pkg/cmd/environment/delete" cmdList "github.com/OctopusDeploy/cli/pkg/cmd/environment/list" + cmdTag "github.com/OctopusDeploy/cli/pkg/cmd/environment/tag" "github.com/OctopusDeploy/cli/pkg/constants" "github.com/OctopusDeploy/cli/pkg/constants/annotations" "github.com/OctopusDeploy/cli/pkg/factory" @@ -28,5 +29,6 @@ func NewCmdEnvironment(f factory.Factory) *cobra.Command { cmd.AddCommand(cmdList.NewCmdList(f)) cmd.AddCommand(cmdDelete.NewCmdDelete(f)) cmd.AddCommand(cmdCreate.NewCmdCreate(f)) + cmd.AddCommand(cmdTag.NewCmdTag(f)) return cmd } diff --git a/pkg/cmd/environment/list/list.go b/pkg/cmd/environment/list/list.go index 2509eb56..bd8f1aad 100644 --- a/pkg/cmd/environment/list/list.go +++ b/pkg/cmd/environment/list/list.go @@ -39,13 +39,21 @@ func NewCmdList(f factory.Factory) *cobra.Command { return output.PrintArray(allEnvs, cmd, output.Mappers[*environments.Environment]{ Json: func(item *environments.Environment) any { - return output.IdAndName{Id: item.GetID(), Name: item.Name} + return struct { + Id string `json:"Id"` + Name string `json:"Name"` + EnvironmentTags []string `json:"EnvironmentTags,omitempty"` + }{ + Id: item.GetID(), + Name: item.Name, + EnvironmentTags: item.EnvironmentTags, + } }, Table: output.TableDefinition[*environments.Environment]{ - Header: []string{"NAME", "GUIDED FAILURE"}, + Header: []string{"NAME", "GUIDED FAILURE", "TAGS"}, Row: func(item *environments.Environment) []string { - return []string{output.Bold(item.Name), strconv.FormatBool(item.UseGuidedFailure)} + return []string{output.Bold(item.Name), strconv.FormatBool(item.UseGuidedFailure), output.FormatAsList(item.EnvironmentTags)} }, }, Basic: func(item *environments.Environment) string { diff --git a/pkg/cmd/environment/tag/tag.go b/pkg/cmd/environment/tag/tag.go new file mode 100644 index 00000000..b611cd26 --- /dev/null +++ b/pkg/cmd/environment/tag/tag.go @@ -0,0 +1,137 @@ +package tag + +import ( + "fmt" + + "github.com/MakeNowJust/heredoc/v2" + "github.com/OctopusDeploy/cli/pkg/cmd" + "github.com/OctopusDeploy/cli/pkg/constants" + "github.com/OctopusDeploy/cli/pkg/factory" + "github.com/OctopusDeploy/cli/pkg/question" + "github.com/OctopusDeploy/cli/pkg/question/selectors" + "github.com/OctopusDeploy/cli/pkg/util/flag" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/environments" + "github.com/spf13/cobra" +) + +const ( + FlagTag = "tag" + FlagEnvironment = "environment" +) + +type TagFlags struct { + Tag *flag.Flag[[]string] + Environment *flag.Flag[string] +} + +func NewTagFlags() *TagFlags { + return &TagFlags{ + Tag: flag.New[[]string](FlagTag, false), + Environment: flag.New[string](FlagEnvironment, false), + } +} + +func NewCmdTag(f factory.Factory) *cobra.Command { + createFlags := NewTagFlags() + + cmd := &cobra.Command{ + Use: "tag", + Short: "Override tags for an environment", + Long: "Override tags for an environment in Octopus Deploy", + Example: heredoc.Docf("$ %s environment tag Environment-1", constants.ExecutableName), + RunE: func(c *cobra.Command, _ []string) error { + opts := NewTagOptions(createFlags, cmd.NewDependencies(f, c)) + + return createRun(opts) + }, + } + + flags := cmd.Flags() + flags.StringArrayVarP(&createFlags.Tag.Value, createFlags.Tag.Name, "t", []string{}, "Tag to apply to environment, must use canonical name: /") + flags.StringVar(&createFlags.Environment.Value, createFlags.Environment.Name, "", "Name or ID of the environment you wish to update") + + return cmd +} + +func createRun(opts *TagOptions) error { + var optsArray []cmd.Dependable + var err error + if !opts.NoPrompt { + optsArray, err = PromptMissing(opts) + if err != nil { + return err + } + } else { + // Validate tags when running with --no-prompt + if len(opts.Tag.Value) > 0 { + tagSets, err := opts.GetAllTagsCallback() + if err != nil { + return err + } + if err := selectors.ValidateTags(opts.Tag.Value, tagSets); err != nil { + return err + } + } + optsArray = append(optsArray, opts) + } + + for _, o := range optsArray { + if err := o.Commit(); err != nil { + return err + } + } + + if !opts.NoPrompt { + fmt.Fprintln(opts.Out, "\nAutomation Commands:") + for _, o := range optsArray { + o.GenerateAutomationCmd() + } + } + + return nil +} + +func PromptMissing(opts *TagOptions) ([]cmd.Dependable, error) { + nestedOpts := []cmd.Dependable{} + + environment, err := AskEnvironments(opts.Ask, opts.Environment.Value, opts.GetEnvironmentsCallback, opts.GetEnvironmentCallback) + if err != nil { + return nil, err + } + opts.environment = environment + opts.Environment.Value = environment.Name + + tagSets, err := opts.GetAllTagsCallback() + if err != nil { + return nil, err + } + + tags, err := selectors.Tags(opts.Ask, opts.environment.EnvironmentTags, opts.Tag.Value, tagSets) + if err != nil { + return nil, err + } + opts.Tag.Value = tags + + nestedOpts = append(nestedOpts, opts) + return nestedOpts, nil +} + +func AskEnvironments(ask question.Asker, value string, getEnvironmentsCallback GetEnvironmentsCallback, getEnvironmentCallback GetEnvironmentCallback) (*environments.Environment, error) { + if value != "" { + environment, err := getEnvironmentCallback(value) + if err != nil { + return nil, err + } + return environment, nil + } + + environment, err := selectors.Select(ask, "Select the Environment you would like to update", getEnvironmentsCallback, func(item *environments.Environment) string { + return item.Name + }) + if err != nil { + return nil, err + } + + return environment, nil +} + diff --git a/pkg/cmd/environment/tag/tag_opts.go b/pkg/cmd/environment/tag/tag_opts.go new file mode 100644 index 00000000..924a3d61 --- /dev/null +++ b/pkg/cmd/environment/tag/tag_opts.go @@ -0,0 +1,116 @@ +package tag + +import ( + "fmt" + "strings" + + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/client" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/environments" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/tagsets" + + "github.com/OctopusDeploy/cli/pkg/cmd" + "github.com/OctopusDeploy/cli/pkg/util/flag" +) + +type GetAllTagSetsCallback func() ([]*tagsets.TagSet, error) +type GetEnvironmentCallback func(environmentIdentifier string) (*environments.Environment, error) +type GetEnvironmentsCallback func() ([]*environments.Environment, error) + +type TagOptions struct { + *TagFlags + *cmd.Dependencies + GetAllTagsCallback GetAllTagSetsCallback + GetEnvironmentCallback GetEnvironmentCallback + GetEnvironmentsCallback GetEnvironmentsCallback + environment *environments.Environment +} + +func NewTagOptions(tagFlags *TagFlags, dependencies *cmd.Dependencies) *TagOptions { + return &TagOptions{ + TagFlags: tagFlags, + Dependencies: dependencies, + GetAllTagsCallback: getAllTagSetsCallback(dependencies.Client), + GetEnvironmentCallback: getEnvironmentCallback(dependencies.Client), + GetEnvironmentsCallback: getEnvironmentsCallback(dependencies.Client), + environment: nil, + } +} + +func (to *TagOptions) Commit() error { + if to.environment == nil { + environment, err := to.GetEnvironmentCallback(to.Environment.Value) + if err != nil { + return err + } + to.environment = environment + } + + to.environment.EnvironmentTags = to.Tag.Value + + updatedEnvironment, err := to.Client.Environments.Update(to.environment) + if err != nil { + return err + } + + _, err = fmt.Fprintf(to.Out, "\nSuccessfully updated environment %s (%s).\n", updatedEnvironment.Name, updatedEnvironment.ID) + if err != nil { + return err + } + return nil +} + +func (to *TagOptions) GenerateAutomationCmd() { + if !to.NoPrompt { + autoCmd := flag.GenerateAutomationCmd(to.CmdPath, to.Environment, to.Tag) + fmt.Fprintf(to.Out, "%s\n", autoCmd) + } +} + +func getEnvironmentCallback(client *client.Client) GetEnvironmentCallback { + return func(environmentIdentifier string) (*environments.Environment, error) { + // Try to get by ID first + environment, _ := environments.GetByID(client, client.GetSpaceID(), environmentIdentifier) + if environment != nil { + return environment, nil + } + + // Fall back to lookup by name + allEnvironments, err := environments.Get(client, client.GetSpaceID(), environments.EnvironmentsQuery{ + PartialName: environmentIdentifier, + }) + if err != nil { + return nil, err + } + + for _, env := range allEnvironments.Items { + if strings.EqualFold(env.Name, environmentIdentifier) { + return env, nil + } + } + + return nil, fmt.Errorf("environment '%s' not found", environmentIdentifier) + } +} + +func getEnvironmentsCallback(client *client.Client) GetEnvironmentsCallback { + return func() ([]*environments.Environment, error) { + allEnvironments, err := environments.GetAll(client, client.GetSpaceID()) + if err != nil { + return nil, err + } + return allEnvironments, nil + } +} + +func getAllTagSetsCallback(client *client.Client) GetAllTagSetsCallback { + return func() ([]*tagsets.TagSet, error) { + query := tagsets.TagSetsQuery{ + Scopes: []string{string(tagsets.TagSetScopeEnvironment)}, + } + result, err := tagsets.Get(client, client.GetSpaceID(), query) + if err != nil { + return nil, err + } + return result.Items, nil + } +} diff --git a/pkg/cmd/tenant/create/create.go b/pkg/cmd/tenant/create/create.go index 5d17500a..d9a64b8f 100644 --- a/pkg/cmd/tenant/create/create.go +++ b/pkg/cmd/tenant/create/create.go @@ -3,12 +3,12 @@ package create import ( "fmt" - "github.com/AlecAivazis/survey/v2" "github.com/MakeNowJust/heredoc/v2" "github.com/OctopusDeploy/cli/pkg/cmd" "github.com/OctopusDeploy/cli/pkg/constants" "github.com/OctopusDeploy/cli/pkg/factory" "github.com/OctopusDeploy/cli/pkg/question" + "github.com/OctopusDeploy/cli/pkg/question/selectors" "github.com/OctopusDeploy/cli/pkg/util/flag" "github.com/spf13/cobra" ) @@ -66,6 +66,16 @@ func createRun(opts *CreateOptions) error { return err } } else { + // Validate tags when running with --no-prompt + if len(opts.Tag.Value) > 0 { + tagSets, err := opts.GetAllTagsCallback() + if err != nil { + return err + } + if err := selectors.ValidateTags(opts.Tag.Value, tagSets); err != nil { + return err + } + } optsArray = append(optsArray, opts) } @@ -91,38 +101,18 @@ func PromptMissing(opts *CreateOptions) ([]cmd.Dependable, error) { question.AskName(opts.Ask, "", "tenant", &opts.Name.Value) question.AskDescription(opts.Ask, "", "tenant", &opts.Description.Value) - tags, err := AskTags(opts.Ask, opts.Tag.Value, opts.GetAllTagsCallback) + tagSets, err := opts.GetAllTagsCallback() if err != nil { return nil, err } - opts.Tag.Value = tags - - nestedOpts = append(nestedOpts, opts) - return nestedOpts, nil -} -func AskTags(ask question.Asker, value []string, getAllTagSetsCallback GetAllTagSetsCallback) ([]string, error) { - if len(value) > 0 { - return value, nil - } - tagSets, err := getAllTagSetsCallback() + tags, err := selectors.Tags(opts.Ask, []string{}, opts.Tag.Value, tagSets) if err != nil { return nil, err } + opts.Tag.Value = tags - canonicalTagName := []string{} - for _, tagSet := range tagSets { - for _, tag := range tagSet.Tags { - canonicalTagName = append(canonicalTagName, tag.CanonicalTagName) - } - } - tags := []string{} - err = ask(&survey.MultiSelect{ - Options: canonicalTagName, - Message: "Tags", - }, &tags) - if err != nil { - return nil, err - } - return tags, nil + nestedOpts = append(nestedOpts, opts) + return nestedOpts, nil } + diff --git a/pkg/cmd/tenant/create/create_opts.go b/pkg/cmd/tenant/create/create_opts.go index 22501adb..41f5b5a5 100644 --- a/pkg/cmd/tenant/create/create_opts.go +++ b/pkg/cmd/tenant/create/create_opts.go @@ -59,10 +59,13 @@ func (co *CreateOptions) GenerateAutomationCmd() { func getAllTagSetsCallback(client *client.Client) GetAllTagSetsCallback { return func() ([]*tagsets.TagSet, error) { - tagSets, err := client.TagSets.GetAll() + query := tagsets.TagSetsQuery{ + Scopes: []string{string(tagsets.TagSetScopeTenant)}, + } + result, err := tagsets.Get(client, client.GetSpaceID(), query) if err != nil { return nil, err } - return tagSets, nil + return result.Items, nil } } diff --git a/pkg/cmd/tenant/create/create_test.go b/pkg/cmd/tenant/create/create_test.go index eaf27f9c..78bc825e 100644 --- a/pkg/cmd/tenant/create/create_test.go +++ b/pkg/cmd/tenant/create/create_test.go @@ -3,39 +3,55 @@ package create_test import ( "testing" - tenantCreate "github.com/OctopusDeploy/cli/pkg/cmd/tenant/create" + "github.com/AlecAivazis/survey/v2" + "github.com/OctopusDeploy/cli/pkg/question/selectors" "github.com/OctopusDeploy/cli/test/testutil" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/tagsets" + "github.com/stretchr/testify/assert" ) func TestTenantCreate_Tags(t *testing.T) { - tags := []string{ - "foo/bar", - "foo/car", - "bin/bop", + fooTagSet := tagsets.NewTagSet("foo") + fooTagSet.Type = tagsets.TagSetTypeMultiSelect + fooTagSet.Tags = []*tagsets.Tag{ + tagsets.NewTag("bar", "#000000"), + tagsets.NewTag("car", "#0000FF"), } + fooTagSet.Tags[0].CanonicalTagName = "foo/bar" + fooTagSet.Tags[1].CanonicalTagName = "foo/car" + + binTagSet := tagsets.NewTagSet("bin") + binTagSet.Type = tagsets.TagSetTypeMultiSelect + binTagSet.Tags = []*tagsets.Tag{ + tagsets.NewTag("bop", "#FF0000"), + } + binTagSet.Tags[0].CanonicalTagName = "bin/bop" + + tagSets := []*tagsets.TagSet{fooTagSet, binTagSet} + pa := []*testutil.PA{ - testutil.NewMultiSelectPrompt("Tags", "", tags, []string{"foo/bar"}), + { + Prompt: &survey.MultiSelect{ + Message: "foo", + Options: []string{"foo/bar", "foo/car"}, + Default: []string{}, + }, + Answer: []string{"foo/bar"}, + }, + { + Prompt: &survey.MultiSelect{ + Message: "bin", + Options: []string{"bin/bop"}, + Default: []string{}, + }, + Answer: []string{}, + }, } asker, checkRemainingPrompts := testutil.NewMockAsker(t, pa) - getAllTagsCallback := func() ([]*tagsets.TagSet, error) { - fooTagSet := tagsets.NewTagSet("foo") - fooTagSet.Tags = []*tagsets.Tag{ - tagsets.NewTag("bar", "#000000"), - tagsets.NewTag("car", "#0000FF"), - } - fooTagSet.Tags[0].CanonicalTagName = "foo/bar" - fooTagSet.Tags[1].CanonicalTagName = "foo/car" - binTagSet := tagsets.NewTagSet("bin") - binTagSet.Tags = []*tagsets.Tag{ - tagsets.NewTag("bop", "#FF0000"), - } - binTagSet.Tags[0].CanonicalTagName = "bin/bop" - return []*tagsets.TagSet{ - fooTagSet, - binTagSet, - }, nil - } - tenantCreate.AskTags(asker, []string{}, getAllTagsCallback) + + result, err := selectors.Tags(asker, []string{}, []string{}, tagSets) + checkRemainingPrompts() + assert.Nil(t, err) + assert.Equal(t, []string{"foo/bar"}, result) } diff --git a/pkg/cmd/tenant/tag/tag.go b/pkg/cmd/tenant/tag/tag.go index 6d27d30f..b3a7396d 100644 --- a/pkg/cmd/tenant/tag/tag.go +++ b/pkg/cmd/tenant/tag/tag.go @@ -3,7 +3,6 @@ package tag import ( "fmt" - "github.com/AlecAivazis/survey/v2" "github.com/MakeNowJust/heredoc/v2" "github.com/OctopusDeploy/cli/pkg/cmd" "github.com/OctopusDeploy/cli/pkg/constants" @@ -63,6 +62,16 @@ func createRun(opts *TagOptions) error { return err } } else { + // Validate tags when running with --no-prompt + if len(opts.Tag.Value) > 0 { + tagSets, err := opts.GetAllTagsCallback() + if err != nil { + return err + } + if err := selectors.ValidateTags(opts.Tag.Value, tagSets); err != nil { + return err + } + } optsArray = append(optsArray, opts) } @@ -92,7 +101,12 @@ func PromptMissing(opts *TagOptions) ([]cmd.Dependable, error) { opts.tenant = tenant opts.Tenant.Value = tenant.Name - tags, err := AskTags(opts.Ask, opts.tenant.TenantTags, opts.Tag.Value, opts.GetAllTagsCallback) + tagSets, err := opts.GetAllTagsCallback() + if err != nil { + return nil, err + } + + tags, err := selectors.Tags(opts.Ask, opts.tenant.TenantTags, opts.Tag.Value, tagSets) if err != nil { return nil, err } @@ -121,29 +135,3 @@ func AskTenants(ask question.Asker, value string, getTenantsCallback GetTenantsC return tenant, nil } -func AskTags(ask question.Asker, value []string, newValue []string, getAllTagSetsCallback GetAllTagSetsCallback) ([]string, error) { - if len(newValue) > 0 { - return newValue, nil - } - tagSets, err := getAllTagSetsCallback() - if err != nil { - return nil, err - } - - canonicalTagName := []string{} - for _, tagSet := range tagSets { - for _, tag := range tagSet.Tags { - canonicalTagName = append(canonicalTagName, tag.CanonicalTagName) - } - } - tags := []string{} - err = ask(&survey.MultiSelect{ - Options: canonicalTagName, - Message: "Tags", - Default: value, - }, &tags) - if err != nil { - return nil, err - } - return tags, nil -} diff --git a/pkg/cmd/tenant/tag/tag_opts.go b/pkg/cmd/tenant/tag/tag_opts.go index 8d176c8a..f1341d12 100644 --- a/pkg/cmd/tenant/tag/tag_opts.go +++ b/pkg/cmd/tenant/tag/tag_opts.go @@ -86,10 +86,13 @@ func getTenantsCallback(client *client.Client) GetTenantsCallback { func getAllTagSetsCallback(client *client.Client) GetAllTagSetsCallback { return func() ([]*tagsets.TagSet, error) { - tagSets, err := client.TagSets.GetAll() + query := tagsets.TagSetsQuery{ + Scopes: []string{string(tagsets.TagSetScopeTenant)}, + } + result, err := tagsets.Get(client, client.GetSpaceID(), query) if err != nil { return nil, err } - return tagSets, nil + return result.Items, nil } } diff --git a/pkg/question/selectors/tags.go b/pkg/question/selectors/tags.go new file mode 100644 index 00000000..1d4cb085 --- /dev/null +++ b/pkg/question/selectors/tags.go @@ -0,0 +1,181 @@ +package selectors + +import ( + "fmt" + "strings" + + "github.com/AlecAivazis/survey/v2" + "github.com/OctopusDeploy/cli/pkg/question" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/tagsets" +) + +// ValidateTags validates that the provided tags follow tag set rules. +// Returns an error if validation fails. +func ValidateTags(tags []string, tagSets []*tagsets.TagSet) error { + for _, tagSet := range tagSets { + if tagSet.Type == tagsets.TagSetTypeSingleSelect { + // Validate that single-select tag sets only have one tag specified + count := 0 + var matchedTags []string + for _, tag := range tags { + for _, t := range tagSet.Tags { + if strings.EqualFold(t.CanonicalTagName, tag) { + count++ + matchedTags = append(matchedTags, tag) + } + } + } + if count > 1 { + return fmt.Errorf("only one tag can be specified for single-select tag set '%s', but found: %v", tagSet.Name, matchedTags) + } + } else if tagSet.Type == tagsets.TagSetTypeFreeText { + // For FreeText, validate that only one tag is specified per tag set + tagSetPrefix := strings.ToLower(tagSet.Name) + "/" + count := 0 + var matchedTags []string + for _, tag := range tags { + if strings.HasPrefix(strings.ToLower(tag), tagSetPrefix) { + // Validate that the tag has content after the prefix + if len(tag) <= len(tagSetPrefix) { + return fmt.Errorf("free text tag '%s' for tag set '%s' must have a value after the prefix", tag, tagSet.Name) + } + count++ + matchedTags = append(matchedTags, tag) + } + } + if count > 1 { + return fmt.Errorf("only one tag can be specified for free text tag set '%s', but found: %v", tagSet.Name, matchedTags) + } + } + } + return nil +} + +// Tags prompts the user to select tags from the provided tag sets. +// For single-select tag sets, a "(None)" option is provided to make selection optional. +// For multi-select tag sets, users can select zero or more tags. +// For free-text tag sets, users can enter custom text which will be prefixed with the tag set name. +// +// Returns the selected canonical tag names. +func Tags(ask question.Asker, currentTags []string, newTags []string, tagSets []*tagsets.TagSet) ([]string, error) { + // If tags were provided via command line, validate and use those + if len(newTags) > 0 { + if err := ValidateTags(newTags, tagSets); err != nil { + return nil, err + } + return newTags, nil + } + + var selectedTags []string + + // Prompt for each tag set + for _, tagSet := range tagSets { + // Find current value(s) for this tag set + var currentValue string + var currentValues []string + tagSetPrefix := strings.ToLower(tagSet.Name) + "/" + for _, v := range currentTags { + // For FreeText, match by prefix; for others, match against tag list + if tagSet.Type == tagsets.TagSetTypeFreeText { + if strings.HasPrefix(strings.ToLower(v), tagSetPrefix) { + currentValue = v + currentValues = append(currentValues, v) + } + } else { + for _, tag := range tagSet.Tags { + if strings.EqualFold(tag.CanonicalTagName, v) { + currentValue = v + currentValues = append(currentValues, v) + } + } + } + } + + if tagSet.Type == tagsets.TagSetTypeFreeText { + // Free text input + var input string + message := tagSet.Name + " (Free Text)" + help := fmt.Sprintf("Enter a free text value for %s tag set", tagSet.Name) + + // If there's a current value, indicate it in the message and help + if currentValue != "" && strings.HasPrefix(strings.ToLower(currentValue), tagSetPrefix) { + currentText := currentValue[len(tagSetPrefix):] + message = fmt.Sprintf("%s [current: %s]", message, currentText) + help = fmt.Sprintf("Current value: %s. Enter new value or leave empty to remove", currentText) + } + + inputPrompt := &survey.Input{ + Message: message, + Help: help, + } + + err := ask(inputPrompt, &input) + if err != nil { + return nil, err + } + // Only add if not empty - empty input removes the tag + if strings.TrimSpace(input) != "" { + canonicalTag := tagSet.Name + "/" + strings.TrimSpace(input) + selectedTags = append(selectedTags, canonicalTag) + } + } else if tagSet.Type == tagsets.TagSetTypeSingleSelect { + // Skip empty tag sets for single/multi select + if len(tagSet.Tags) == 0 { + continue + } + + var tagOptions []string + for _, tag := range tagSet.Tags { + tagOptions = append(tagOptions, tag.CanonicalTagName) + } + // Single select + optionsWithNone := append([]string{"(None)"}, tagOptions...) + var selected string + selectPrompt := &survey.Select{ + Options: optionsWithNone, + Message: tagSet.Name + " (Single Select)", + } + if currentValue != "" { + selectPrompt.Default = currentValue + } else { + selectPrompt.Default = "(None)" + } + err := ask(selectPrompt, &selected) + if err != nil { + return nil, err + } + // Only add if not "None" + if selected != "" && selected != "(None)" { + selectedTags = append(selectedTags, selected) + } + } else { + // Multi select + // Skip empty tag sets for multi select + if len(tagSet.Tags) == 0 { + continue + } + + var tagOptions []string + for _, tag := range tagSet.Tags { + tagOptions = append(tagOptions, tag.CanonicalTagName) + } + + var selected []string + defaultValues := currentValues + if defaultValues == nil { + defaultValues = []string{} + } + err := ask(&survey.MultiSelect{ + Options: tagOptions, + Message: tagSet.Name, + Default: defaultValues, + }, &selected) + if err != nil { + return nil, err + } + selectedTags = append(selectedTags, selected...) + } + } + + return selectedTags, nil +} diff --git a/pkg/question/selectors/tags_test.go b/pkg/question/selectors/tags_test.go new file mode 100644 index 00000000..268a525f --- /dev/null +++ b/pkg/question/selectors/tags_test.go @@ -0,0 +1,197 @@ +package selectors + +import ( + "testing" + + "github.com/OctopusDeploy/cli/test/testutil" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/tagsets" + "github.com/stretchr/testify/assert" +) + +func createTestTagSets() []*tagsets.TagSet { + // Single-select tag set + regionTagSet := tagsets.NewTagSet("Region") + regionTagSet.Type = tagsets.TagSetTypeSingleSelect + regionTagSet.Tags = []*tagsets.Tag{ + tagsets.NewTag("US East", "#000000"), + tagsets.NewTag("US West", "#0000FF"), + } + regionTagSet.Tags[0].CanonicalTagName = "region/us-east" + regionTagSet.Tags[1].CanonicalTagName = "region/us-west" + + // Multi-select tag set + envTypeTagSet := tagsets.NewTagSet("Environment Type") + envTypeTagSet.Type = tagsets.TagSetTypeMultiSelect + envTypeTagSet.Tags = []*tagsets.Tag{ + tagsets.NewTag("Production", "#FF0000"), + tagsets.NewTag("Staging", "#00FF00"), + } + envTypeTagSet.Tags[0].CanonicalTagName = "envtype/production" + envTypeTagSet.Tags[1].CanonicalTagName = "envtype/staging" + + return []*tagsets.TagSet{regionTagSet, envTypeTagSet} +} + +func TestValidateTags_ValidSingleSelectTag(t *testing.T) { + tagSets := createTestTagSets() + tags := []string{"region/us-east"} + + err := ValidateTags(tags, tagSets) + + assert.Nil(t, err) +} + +func TestValidateTags_ErrorOnMultipleSingleSelectTags(t *testing.T) { + tagSets := createTestTagSets() + tags := []string{"region/us-east", "region/us-west"} + + err := ValidateTags(tags, tagSets) + + assert.NotNil(t, err) + assert.Contains(t, err.Error(), "only one tag can be specified for single-select tag set 'Region'") +} + +func TestValidateTags_ValidMultiSelectTags(t *testing.T) { + tagSets := createTestTagSets() + tags := []string{"envtype/production", "envtype/staging"} + + err := ValidateTags(tags, tagSets) + + assert.Nil(t, err) +} + +func TestValidateTags_MixedTags(t *testing.T) { + tagSets := createTestTagSets() + tags := []string{"region/us-east", "envtype/production", "envtype/staging"} + + err := ValidateTags(tags, tagSets) + + assert.Nil(t, err) +} + +func TestTags_ReturnsProvidedTagsWhenValid(t *testing.T) { + tagSets := createTestTagSets() + providedTags := []string{"region/us-east", "envtype/production"} + + result, err := Tags(nil, []string{}, providedTags, tagSets) + + assert.Nil(t, err) + assert.Equal(t, providedTags, result) +} + +func TestTags_ReturnsErrorWhenProvidedTagsInvalid(t *testing.T) { + tagSets := createTestTagSets() + providedTags := []string{"region/us-east", "region/us-west"} + + result, err := Tags(nil, []string{}, providedTags, tagSets) + + assert.NotNil(t, err) + assert.Nil(t, result) +} + +func TestValidateTags_FreeTextValid(t *testing.T) { + freeTextTagSet := tagsets.NewTagSet("Customer") + freeTextTagSet.Type = tagsets.TagSetTypeFreeText + + tagSets := []*tagsets.TagSet{freeTextTagSet} + tags := []string{"customer/company a"} + + err := ValidateTags(tags, tagSets) + + assert.Nil(t, err) +} + +func TestValidateTags_FreeTextMultipleError(t *testing.T) { + freeTextTagSet := tagsets.NewTagSet("Customer") + freeTextTagSet.Type = tagsets.TagSetTypeFreeText + + tagSets := []*tagsets.TagSet{freeTextTagSet} + tags := []string{"customer/company a", "customer/company b"} + + err := ValidateTags(tags, tagSets) + + assert.NotNil(t, err) + assert.Contains(t, err.Error(), "only one tag can be specified for free text tag set 'Customer'") +} + +func TestValidateTags_FreeTextEmptyValueError(t *testing.T) { + freeTextTagSet := tagsets.NewTagSet("Customer") + freeTextTagSet.Type = tagsets.TagSetTypeFreeText + + tagSets := []*tagsets.TagSet{freeTextTagSet} + tags := []string{"customer/"} + + err := ValidateTags(tags, tagSets) + + assert.NotNil(t, err) + assert.Contains(t, err.Error(), "must have a value after the prefix") +} + +func TestTags_FreeTextReturnsProvidedTags(t *testing.T) { + freeTextTagSet := tagsets.NewTagSet("Customer") + freeTextTagSet.Type = tagsets.TagSetTypeFreeText + + tagSets := []*tagsets.TagSet{freeTextTagSet} + providedTags := []string{"customer/company a"} + + result, err := Tags(nil, []string{}, providedTags, tagSets) + + assert.Nil(t, err) + assert.Equal(t, providedTags, result) +} + +func TestTags_FreeTextPrompt(t *testing.T) { + freeTextTagSet := tagsets.NewTagSet("Customer") + freeTextTagSet.Type = tagsets.TagSetTypeFreeText + + tagSets := []*tagsets.TagSet{freeTextTagSet} + + pa := []*testutil.PA{ + testutil.NewInputPrompt("Customer (Free Text)", "Enter a free text value for Customer tag set", "company a"), + } + asker, checkRemainingPrompts := testutil.NewMockAsker(t, pa) + + result, err := Tags(asker, []string{}, []string{}, tagSets) + + checkRemainingPrompts() + assert.Nil(t, err) + assert.Equal(t, []string{"Customer/company a"}, result) +} + +func TestTags_FreeTextPromptWithCurrentValue(t *testing.T) { + freeTextTagSet := tagsets.NewTagSet("Customer") + freeTextTagSet.Type = tagsets.TagSetTypeFreeText + + tagSets := []*tagsets.TagSet{freeTextTagSet} + currentTags := []string{"customer/old-value"} + + pa := []*testutil.PA{ + testutil.NewInputPrompt("Customer (Free Text) [current: old-value]", "Current value: old-value. Enter new value or leave empty to remove", "new-value"), + } + asker, checkRemainingPrompts := testutil.NewMockAsker(t, pa) + + result, err := Tags(asker, currentTags, []string{}, tagSets) + + checkRemainingPrompts() + assert.Nil(t, err) + assert.Equal(t, []string{"Customer/new-value"}, result) +} + +func TestTags_FreeTextPromptClearsExistingValue(t *testing.T) { + freeTextTagSet := tagsets.NewTagSet("Customer") + freeTextTagSet.Type = tagsets.TagSetTypeFreeText + + tagSets := []*tagsets.TagSet{freeTextTagSet} + currentTags := []string{"customer/old-value"} + + pa := []*testutil.PA{ + testutil.NewInputPrompt("Customer (Free Text) [current: old-value]", "Current value: old-value. Enter new value or leave empty to remove", ""), + } + asker, checkRemainingPrompts := testutil.NewMockAsker(t, pa) + + result, err := Tags(asker, currentTags, []string{}, tagSets) + + checkRemainingPrompts() + assert.Nil(t, err) + assert.Empty(t, result) +} From 45a77d5c8c32f09df232b2b974d876af6e808b36 Mon Sep 17 00:00:00 2001 From: Grace Rehn Date: Wed, 8 Oct 2025 12:42:01 +1000 Subject: [PATCH 2/3] feat: Add tag set scope validation and feedback for single tag/tenant prompt --- pkg/cmd/environment/tag/tag.go | 16 ++++++++++-- pkg/cmd/tenant/tag/tag.go | 16 ++++++++++-- pkg/question/selectors/tags.go | 38 +++++++++++++++++++++++++++++ pkg/question/selectors/tags_test.go | 20 +++++++++++++++ 4 files changed, 86 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/environment/tag/tag.go b/pkg/cmd/environment/tag/tag.go index b611cd26..c6c9595a 100644 --- a/pkg/cmd/environment/tag/tag.go +++ b/pkg/cmd/environment/tag/tag.go @@ -2,11 +2,13 @@ package tag import ( "fmt" + "io" "github.com/MakeNowJust/heredoc/v2" "github.com/OctopusDeploy/cli/pkg/cmd" "github.com/OctopusDeploy/cli/pkg/constants" "github.com/OctopusDeploy/cli/pkg/factory" + "github.com/OctopusDeploy/cli/pkg/output" "github.com/OctopusDeploy/cli/pkg/question" "github.com/OctopusDeploy/cli/pkg/question/selectors" "github.com/OctopusDeploy/cli/pkg/util/flag" @@ -94,7 +96,7 @@ func createRun(opts *TagOptions) error { func PromptMissing(opts *TagOptions) ([]cmd.Dependable, error) { nestedOpts := []cmd.Dependable{} - environment, err := AskEnvironments(opts.Ask, opts.Environment.Value, opts.GetEnvironmentsCallback, opts.GetEnvironmentCallback) + environment, err := AskEnvironments(opts.Ask, opts.Out, opts.Environment.Value, opts.GetEnvironmentsCallback, opts.GetEnvironmentCallback) if err != nil { return nil, err } @@ -116,7 +118,7 @@ func PromptMissing(opts *TagOptions) ([]cmd.Dependable, error) { return nestedOpts, nil } -func AskEnvironments(ask question.Asker, value string, getEnvironmentsCallback GetEnvironmentsCallback, getEnvironmentCallback GetEnvironmentCallback) (*environments.Environment, error) { +func AskEnvironments(ask question.Asker, out io.Writer, value string, getEnvironmentsCallback GetEnvironmentsCallback, getEnvironmentCallback GetEnvironmentCallback) (*environments.Environment, error) { if value != "" { environment, err := getEnvironmentCallback(value) if err != nil { @@ -125,6 +127,16 @@ func AskEnvironments(ask question.Asker, value string, getEnvironmentsCallback G return environment, nil } + // Check if there's only one environment + envs, err := getEnvironmentsCallback() + if err != nil { + return nil, err + } + if len(envs) == 1 { + fmt.Fprintf(out, "Selecting only available environment '%s'.\n", output.Cyan(envs[0].Name)) + return envs[0], nil + } + environment, err := selectors.Select(ask, "Select the Environment you would like to update", getEnvironmentsCallback, func(item *environments.Environment) string { return item.Name }) diff --git a/pkg/cmd/tenant/tag/tag.go b/pkg/cmd/tenant/tag/tag.go index b3a7396d..07b21324 100644 --- a/pkg/cmd/tenant/tag/tag.go +++ b/pkg/cmd/tenant/tag/tag.go @@ -2,11 +2,13 @@ package tag import ( "fmt" + "io" "github.com/MakeNowJust/heredoc/v2" "github.com/OctopusDeploy/cli/pkg/cmd" "github.com/OctopusDeploy/cli/pkg/constants" "github.com/OctopusDeploy/cli/pkg/factory" + "github.com/OctopusDeploy/cli/pkg/output" "github.com/OctopusDeploy/cli/pkg/question" "github.com/OctopusDeploy/cli/pkg/question/selectors" "github.com/OctopusDeploy/cli/pkg/util/flag" @@ -94,7 +96,7 @@ func createRun(opts *TagOptions) error { func PromptMissing(opts *TagOptions) ([]cmd.Dependable, error) { nestedOpts := []cmd.Dependable{} - tenant, err := AskTenants(opts.Ask, opts.Tenant.Value, opts.GetTenantsCallback, opts.GetTenantCallback) + tenant, err := AskTenants(opts.Ask, opts.Out, opts.Tenant.Value, opts.GetTenantsCallback, opts.GetTenantCallback) if err != nil { return nil, err } @@ -116,7 +118,7 @@ func PromptMissing(opts *TagOptions) ([]cmd.Dependable, error) { return nestedOpts, nil } -func AskTenants(ask question.Asker, value string, getTenantsCallback GetTenantsCallback, getTenantCallback GetTenantCallback) (*tenants.Tenant, error) { +func AskTenants(ask question.Asker, out io.Writer, value string, getTenantsCallback GetTenantsCallback, getTenantCallback GetTenantCallback) (*tenants.Tenant, error) { if value != "" { tenant, err := getTenantCallback(value) if err != nil { @@ -125,6 +127,16 @@ func AskTenants(ask question.Asker, value string, getTenantsCallback GetTenantsC return tenant, nil } + // Check if there's only one tenant + tns, err := getTenantsCallback() + if err != nil { + return nil, err + } + if len(tns) == 1 { + fmt.Fprintf(out, "Selecting only available tenant '%s'.\n", output.Cyan(tns[0].Name)) + return tns[0], nil + } + tenant, err := selectors.Select(ask, "Select the Tenant you would like to update", getTenantsCallback, func(item *tenants.Tenant) string { return item.Name }) diff --git a/pkg/question/selectors/tags.go b/pkg/question/selectors/tags.go index 1d4cb085..775f4194 100644 --- a/pkg/question/selectors/tags.go +++ b/pkg/question/selectors/tags.go @@ -12,6 +12,44 @@ import ( // ValidateTags validates that the provided tags follow tag set rules. // Returns an error if validation fails. func ValidateTags(tags []string, tagSets []*tagsets.TagSet) error { + // Validate that each tag belongs to one of the provided tag sets + for _, tag := range tags { + // Extract tag set name from the tag ("TagSetName/TagName") + tagSetName := "" + if idx := strings.Index(tag, "/"); idx > 0 { + tagSetName = tag[:idx] + } else { + return fmt.Errorf("tag '%s' is not in the correct format (expected 'TagSetName/TagName')", tag) + } + + // Find the tag set + var foundTagSet *tagsets.TagSet + for _, ts := range tagSets { + if strings.EqualFold(ts.Name, tagSetName) { + foundTagSet = ts + break + } + } + + if foundTagSet == nil { + return fmt.Errorf("tag '%s' does not belong to any tag set available for this resource", tag) + } + + // For non-FreeText tag sets, validate that the specific tag exists + if foundTagSet.Type != tagsets.TagSetTypeFreeText { + tagFound := false + for _, t := range foundTagSet.Tags { + if strings.EqualFold(t.CanonicalTagName, tag) { + tagFound = true + break + } + } + if !tagFound { + return fmt.Errorf("tag '%s' does not exist in tag set '%s'", tag, foundTagSet.Name) + } + } + } + for _, tagSet := range tagSets { if tagSet.Type == tagsets.TagSetTypeSingleSelect { // Validate that single-select tag sets only have one tag specified diff --git a/pkg/question/selectors/tags_test.go b/pkg/question/selectors/tags_test.go index 268a525f..5b4fa86a 100644 --- a/pkg/question/selectors/tags_test.go +++ b/pkg/question/selectors/tags_test.go @@ -195,3 +195,23 @@ func TestTags_FreeTextPromptClearsExistingValue(t *testing.T) { assert.Nil(t, err) assert.Empty(t, result) } + +func TestValidateTags_ErrorOnTagNotBelongingToAvailableTagSet(t *testing.T) { + tagSets := createTestTagSets() + tags := []string{"department/engineering"} + + err := ValidateTags(tags, tagSets) + + assert.NotNil(t, err) + assert.Contains(t, err.Error(), "does not belong to any tag set available for this resource") +} + +func TestValidateTags_ErrorOnTagNotExistingInTagSet(t *testing.T) { + tagSets := createTestTagSets() + tags := []string{"region/eu-west"} + + err := ValidateTags(tags, tagSets) + + assert.NotNil(t, err) + assert.Contains(t, err.Error(), "does not exist in tag set") +} From b36d874d47662bacbcff3960b9dae12ec97519c5 Mon Sep 17 00:00:00 2001 From: Grace Rehn Date: Wed, 8 Oct 2025 12:56:40 +1000 Subject: [PATCH 3/3] fix: tags in tests were incorrect --- pkg/question/selectors/tags_test.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/pkg/question/selectors/tags_test.go b/pkg/question/selectors/tags_test.go index 5b4fa86a..ef593b29 100644 --- a/pkg/question/selectors/tags_test.go +++ b/pkg/question/selectors/tags_test.go @@ -16,8 +16,8 @@ func createTestTagSets() []*tagsets.TagSet { tagsets.NewTag("US East", "#000000"), tagsets.NewTag("US West", "#0000FF"), } - regionTagSet.Tags[0].CanonicalTagName = "region/us-east" - regionTagSet.Tags[1].CanonicalTagName = "region/us-west" + regionTagSet.Tags[0].CanonicalTagName = "Region/US East" + regionTagSet.Tags[1].CanonicalTagName = "Region/US West" // Multi-select tag set envTypeTagSet := tagsets.NewTagSet("Environment Type") @@ -26,15 +26,15 @@ func createTestTagSets() []*tagsets.TagSet { tagsets.NewTag("Production", "#FF0000"), tagsets.NewTag("Staging", "#00FF00"), } - envTypeTagSet.Tags[0].CanonicalTagName = "envtype/production" - envTypeTagSet.Tags[1].CanonicalTagName = "envtype/staging" + envTypeTagSet.Tags[0].CanonicalTagName = "Environment Type/Production" + envTypeTagSet.Tags[1].CanonicalTagName = "Environment Type/Staging" return []*tagsets.TagSet{regionTagSet, envTypeTagSet} } func TestValidateTags_ValidSingleSelectTag(t *testing.T) { tagSets := createTestTagSets() - tags := []string{"region/us-east"} + tags := []string{"Region/US East"} err := ValidateTags(tags, tagSets) @@ -43,7 +43,7 @@ func TestValidateTags_ValidSingleSelectTag(t *testing.T) { func TestValidateTags_ErrorOnMultipleSingleSelectTags(t *testing.T) { tagSets := createTestTagSets() - tags := []string{"region/us-east", "region/us-west"} + tags := []string{"Region/US East", "Region/US West"} err := ValidateTags(tags, tagSets) @@ -53,7 +53,7 @@ func TestValidateTags_ErrorOnMultipleSingleSelectTags(t *testing.T) { func TestValidateTags_ValidMultiSelectTags(t *testing.T) { tagSets := createTestTagSets() - tags := []string{"envtype/production", "envtype/staging"} + tags := []string{"Environment Type/Production", "Environment Type/Staging"} err := ValidateTags(tags, tagSets) @@ -62,7 +62,7 @@ func TestValidateTags_ValidMultiSelectTags(t *testing.T) { func TestValidateTags_MixedTags(t *testing.T) { tagSets := createTestTagSets() - tags := []string{"region/us-east", "envtype/production", "envtype/staging"} + tags := []string{"Region/US East", "Environment Type/Production", "Environment Type/Staging"} err := ValidateTags(tags, tagSets) @@ -71,7 +71,7 @@ func TestValidateTags_MixedTags(t *testing.T) { func TestTags_ReturnsProvidedTagsWhenValid(t *testing.T) { tagSets := createTestTagSets() - providedTags := []string{"region/us-east", "envtype/production"} + providedTags := []string{"Region/US East", "Environment Type/Production"} result, err := Tags(nil, []string{}, providedTags, tagSets) @@ -81,7 +81,7 @@ func TestTags_ReturnsProvidedTagsWhenValid(t *testing.T) { func TestTags_ReturnsErrorWhenProvidedTagsInvalid(t *testing.T) { tagSets := createTestTagSets() - providedTags := []string{"region/us-east", "region/us-west"} + providedTags := []string{"Region/US East", "Region/US West"} result, err := Tags(nil, []string{}, providedTags, tagSets) @@ -208,7 +208,7 @@ func TestValidateTags_ErrorOnTagNotBelongingToAvailableTagSet(t *testing.T) { func TestValidateTags_ErrorOnTagNotExistingInTagSet(t *testing.T) { tagSets := createTestTagSets() - tags := []string{"region/eu-west"} + tags := []string{"Region/EU West"} err := ValidateTags(tags, tagSets)