From da9a313d0cd45c61641d7585771d93ca503a23c9 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 3 Jun 2025 16:55:34 +0100 Subject: [PATCH] fix(cli): allow specifying empty provisioner tag set with --provisioner-tag="-" (#18205) Relates to https://github.com/coder/coder/issues/17818 Note: due to limitations in `cobra/serpent` I ended up having to use `-` to signify absence of provisioner tags. This value is not a valid key-value pair and thus not a valid tag. --- cli/templatepush.go | 58 ++++++++++----- cli/templatepush_test.go | 71 ++++++++++++++++++- .../coder_templates_push_--help.golden | 5 +- docs/reference/cli/templates_push.md | 2 +- 4 files changed, 117 insertions(+), 19 deletions(-) diff --git a/cli/templatepush.go b/cli/templatepush.go index 6f8edf61b5..312c8a466e 100644 --- a/cli/templatepush.go +++ b/cli/templatepush.go @@ -8,6 +8,7 @@ import ( "net/http" "os" "path/filepath" + "slices" "strings" "time" @@ -80,6 +81,46 @@ func (r *RootCmd) templatePush() *serpent.Command { createTemplate = true } + var tags map[string]string + // Passing --provisioner-tag="-" allows the user to clear all provisioner tags. + if len(provisionerTags) == 1 && strings.TrimSpace(provisionerTags[0]) == "-" { + cliui.Warn(inv.Stderr, "Not reusing provisioner tags from the previous template version.") + tags = map[string]string{} + } else { + tags, err = ParseProvisionerTags(provisionerTags) + if err != nil { + return err + } + + // If user hasn't provided new provisioner tags, inherit ones from the active template version. + if len(tags) == 0 && template.ActiveVersionID != uuid.Nil { + templateVersion, err := client.TemplateVersion(inv.Context(), template.ActiveVersionID) + if err != nil { + return err + } + tags = templateVersion.Job.Tags + cliui.Info(inv.Stderr, "Re-using provisioner tags from the active template version.") + cliui.Info(inv.Stderr, "Tip: You can override these tags by passing "+cliui.Code(`--provisioner-tag="key=value"`)+".") + cliui.Info(inv.Stderr, " You can also clear all provisioner tags by passing "+cliui.Code(`--provisioner-tag="-"`)+".") + } + } + + { // For clarity, display provisioner tags to the user. + var tmp []string + for k, v := range tags { + if k == provisionersdk.TagScope || k == provisionersdk.TagOwner { + continue + } + tmp = append(tmp, fmt.Sprintf("%s=%q", k, v)) + } + slices.Sort(tmp) + tagStr := strings.Join(tmp, " ") + if len(tmp) == 0 { + tagStr = "" + } + cliui.Info(inv.Stderr, "Provisioner tags: "+cliui.Code(tagStr)) + } + err = uploadFlags.checkForLockfile(inv) if err != nil { return xerrors.Errorf("check for lockfile: %w", err) @@ -104,21 +145,6 @@ func (r *RootCmd) templatePush() *serpent.Command { return err } - tags, err := ParseProvisionerTags(provisionerTags) - if err != nil { - return err - } - - // If user hasn't provided new provisioner tags, inherit ones from the active template version. - if len(tags) == 0 && template.ActiveVersionID != uuid.Nil { - templateVersion, err := client.TemplateVersion(inv.Context(), template.ActiveVersionID) - if err != nil { - return err - } - tags = templateVersion.Job.Tags - inv.Logger.Info(inv.Context(), "reusing existing provisioner tags", "tags", tags) - } - userVariableValues, err := codersdk.ParseUserVariableValues( varsFiles, variablesFile, @@ -214,7 +240,7 @@ func (r *RootCmd) templatePush() *serpent.Command { }, { Flag: "provisioner-tag", - Description: "Specify a set of tags to target provisioner daemons.", + Description: "Specify a set of tags to target provisioner daemons. If you do not specify any tags, the tags from the active template version will be reused, if available. To remove existing tags, use --provisioner-tag=\"-\".", Value: serpent.StringArrayOf(&provisionerTags), }, { diff --git a/cli/templatepush_test.go b/cli/templatepush_test.go index b8e4147e6b..e1a7e612f4 100644 --- a/cli/templatepush_test.go +++ b/cli/templatepush_test.go @@ -602,7 +602,7 @@ func TestTemplatePush(t *testing.T) { templateVersion = coderdtest.AwaitTemplateVersionJobCompleted(t, client, templateVersion.ID) template := coderdtest.CreateTemplate(t, client, owner.OrganizationID, templateVersion.ID) - // Push new template version without provisioner tags. CLI should reuse tags from the previous version. + // Push new template version with different provisioner tags. source := clitest.CreateTemplateVersionSource(t, &echo.Responses{ Parse: echo.ParseComplete, ProvisionApply: echo.ApplyComplete, @@ -639,6 +639,75 @@ func TestTemplatePush(t *testing.T) { require.EqualValues(t, map[string]string{"foobar": "foobaz", "owner": "", "scope": "organization"}, templateVersion.Job.Tags) }) + t.Run("DeleteTags", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + + // Start the first provisioner with no tags. + client, provisionerDocker, api := coderdtest.NewWithAPI(t, &coderdtest.Options{ + IncludeProvisionerDaemon: true, + ProvisionerDaemonTags: map[string]string{}, + }) + defer provisionerDocker.Close() + + // Start the second provisioner with a tag set. + provisionerFoobar := coderdtest.NewTaggedProvisionerDaemon(t, api, "provisioner-foobar", map[string]string{ + "foobar": "foobaz", + }) + defer provisionerFoobar.Close() + + owner := coderdtest.CreateFirstUser(t, client) + templateAdmin, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.RoleTemplateAdmin()) + + // Create the template with initial tagged template version. + templateVersion := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, nil, func(ctvr *codersdk.CreateTemplateVersionRequest) { + ctvr.ProvisionerTags = map[string]string{ + "foobar": "foobaz", + } + }) + templateVersion = coderdtest.AwaitTemplateVersionJobCompleted(t, client, templateVersion.ID) + template := coderdtest.CreateTemplate(t, client, owner.OrganizationID, templateVersion.ID) + + // Stop the tagged provisioner daemon. + provisionerFoobar.Close() + + // Push new template version with no provisioner tags. + source := clitest.CreateTemplateVersionSource(t, &echo.Responses{ + Parse: echo.ParseComplete, + ProvisionApply: echo.ApplyComplete, + }) + inv, root := clitest.New(t, "templates", "push", template.Name, "--directory", source, "--test.provisioner", string(database.ProvisionerTypeEcho), "--name", template.Name, "--provisioner-tag=\"-\"") + clitest.SetupConfig(t, templateAdmin, root) + pty := ptytest.New(t).Attach(inv) + + execDone := make(chan error) + go func() { + execDone <- inv.WithContext(ctx).Run() + }() + + matches := []struct { + match string + write string + }{ + {match: "Upload", write: "yes"}, + } + for _, m := range matches { + pty.ExpectMatch(m.match) + pty.WriteLine(m.write) + } + + require.NoError(t, <-execDone) + + // Verify template version tags + template, err := client.Template(ctx, template.ID) + require.NoError(t, err) + + templateVersion, err = client.TemplateVersion(ctx, template.ActiveVersionID) + require.NoError(t, err) + require.EqualValues(t, map[string]string{"owner": "", "scope": "organization"}, templateVersion.Job.Tags) + }) + t.Run("DoNotChangeTags", func(t *testing.T) { t.Parallel() diff --git a/cli/testdata/coder_templates_push_--help.golden b/cli/testdata/coder_templates_push_--help.golden index eee0ad34ca..edab61a3c5 100644 --- a/cli/testdata/coder_templates_push_--help.golden +++ b/cli/testdata/coder_templates_push_--help.golden @@ -33,7 +33,10 @@ OPTIONS: generated if not provided. --provisioner-tag string-array - Specify a set of tags to target provisioner daemons. + Specify a set of tags to target provisioner daemons. If you do not + specify any tags, the tags from the active template version will be + reused, if available. To remove existing tags, use + --provisioner-tag="-". --var string-array Alias of --variable. diff --git a/docs/reference/cli/templates_push.md b/docs/reference/cli/templates_push.md index 46687d3fc6..8c7901e86e 100644 --- a/docs/reference/cli/templates_push.md +++ b/docs/reference/cli/templates_push.md @@ -41,7 +41,7 @@ Alias of --variable. |------|---------------------------| | Type | string-array | -Specify a set of tags to target provisioner daemons. +Specify a set of tags to target provisioner daemons. If you do not specify any tags, the tags from the active template version will be reused, if available. To remove existing tags, use --provisioner-tag="-". ### --name