cli: add support for environment tags #345

dschaller · 2024-06-25T23:26:19Z

add new CLI commands for CRUD operations against environment tags

> esc env tag ls localumi/env-a

This command lists all tags on the environment in the format below
Name: <tag name>
Value: <tag value>
Last updated at <timestamp> by <user>

> esc env tag localumi/env-b owner dschaller
This command adds a new tag to the specified environment and returns the new tag output similar to the format from list.
If there is already a tag with the specified key (e.g. owner) it will update the value to the new value.

Optionally, users can pass in another argument to specify an update to the key.
> esc env tag localumi/env-b owner Owner dschaller

> esc env tag rm localumi/env-b owner
This command removes a tag from the specified environment with the specified name

dschaller · 2024-06-26T00:42:12Z

Also open to the idea of esc env tag rm and esc env tag update by ID both taking another argument as the tag or tag ID if it makes more sense

pgavlin · 2024-06-26T22:45:42Z

The tag IDs are a bit surprising. We don't typically expose IDs like this--I would have expected this to act exactly like stack tags.

infrastructure ❯ pulumi stack tag   
Manage stack tags

Stacks have associated metadata in the form of tags. Each tag consists of a name
and value. The `get`, `ls`, `rm`, and `set` commands can be used to manage tags.
Some tags are automatically assigned based on the environment each time a stack
is updated.

Usage:
  pulumi stack tag [command]

Available Commands:
  get         Get a single stack tag value
  ls          List all stack tags
  rm          Remove a stack tag
  set         Set a stack tag

Flags:
  -h, --help           help for tag
  -s, --stack string   The name of the stack to operate on. Defaults to the current stack

Global Flags:
      --color string                 Colorize output. Choices are: always, never, raw, auto (default "auto")
  -C, --cwd string                   Run pulumi as if it had been started in another directory
      --disable-integrity-checking   Disable integrity checking of checkpoint files
  -e, --emoji                        Enable emojis in the output (default true)
  -Q, --fully-qualify-stack-names    Show fully-qualified stack names
      --logflow                      Flow log settings to child processes (like plugins)
      --logtostderr                  Log to stderr instead of to files
      --memprofilerate int           Enable more precise (and expensive) memory allocation profiles by setting runtime.MemProfileRate
      --non-interactive              Disable interactive mode for all commands
      --profiling string             Emit CPU and memory profiles and an execution trace to '[filename].[pid].{cpu,mem,trace}', respectively
      --tracing file:                Emit tracing to the specified endpoint. Use the file: scheme to write tracing data to a local file
  -v, --verbose int                  Enable verbose logging (e.g., v=3); anything >3 is very verbose

Use "pulumi stack tag [command] --help" for more information about a command.

dschaller · 2024-06-27T22:52:52Z

👍 I can change the behavior to mirror stack tags.

I intentionally made some tweaks to these commands vs the stack tags for a few reasons:

I exposed the tag IDs to allow for updating the tag name and to handle a scenario where we potentially allow for stack tags with duplicate keys (there was some debate over things like team where a environment might be used by multiple teams)
I kept the tag a single argument with a reserved delimiter (:) to allow for flexibility if we want to support bulk tagging from a single command
Thinking more about it though I'm not really a fan of having the ID or tag name in the path so I will definitely update any of those patterns to move it out into an argument

dschaller · 2024-07-09T23:36:19Z

@pgavlin updated these commands to more closely align with stack tags

we no longer expose the tag ID anywhere
the tag identifier has been moved out of the project/env path anywhere it was kept
moved the tag key/value to be two separate arguments - I really do think we should have a way to do this as a single argument as it allows for bulk tagging but at the moment we don't have a good delimiter that cannot be used in the tag name (:) is a valid tag name character :(

As an aside there is also a fix to show local time if the UTC flag is not set since most of the timestamps coming back from the server are UTC

cmd/esc/cli/env_tag.go

pgavlin · 2024-07-19T23:55:41Z

Could you update the PR description with the new command structure? Also--while the example output is nice, it's a bit tough to parse. I think I'd prefer a brief plain-english description of what each command does to the output.

dschaller · 2024-07-23T17:17:21Z

@pgavlin updated the description

pgavlin · 2024-07-24T17:31:25Z

cmd/esc/cli/cli_test.go

@@ -627,6 +627,86 @@ func (c *testPulumiClient) GetOpenProperty(ctx context.Context, orgName, envName
 	return nil, errors.New("NYI")
 }

+func (c *testPulumiClient) GetEnvironmentTag(


Can you add some tests?

If you add a new file in cli/testdata, it will be automatically picked up by the test harness:

The run section contains the command(s) to run

The environments section contains the environments to define for testing

Once you've filled in those two sections, you can generate the stdout/stderr sections by running PULUMI_ACCEPT=1 go test ./cmd/esc/cli from the repository root.

You'll likely need to revert changes to cmd/esc/cli/testdata/env-login-gh100.yaml and cmd/esc/cli/testdata/env-set.yaml as the regeneration process makes undesirable formatting changes to those files.

cmd/esc/cli/cli_test.go

pgavlin · 2024-07-29T22:08:42Z

cmd/esc/cli/cli_test.go

+		var tags cliTestcaseEnvironmentTags
+		envTags := map[string]string{}
+		if err := env.Decode(&tags); err == nil {
+			envTags = tags.Tags
+		}


I don't think that this is what we want. I think that we should add a new field to cliTestcaseRevisions that contains the environment tags. That will give any test the ability to use environment tags as well as other features.

Sorry do you mean moving the existing tags to cliTestcaseRevisions instead of renaming them to revisionTags on the test environment?

Yeah--it looks like as written an environment can either contain a set of tags or a set of revisions (unless I'm reading it wrong?). Feels like instead we want cliTestcaseRevisions to contain a field of type cliTestcaseEnvironmentTags.

ah yeah I follow you - will update

actually thinking more about this I feel like it makes sense to keep these at the top level on the env since revision tags must be unique across all revisions. I did adjust the interface of revision tags though to allow for passing a list instead of a single value since that mirrors what the API supports

cmd/esc/cli/testdata/env-tag-ls.yaml

cmd/esc/cli/testdata/env-tag-rm.yaml

cmd/esc/cli/testdata/env-tag-set.yaml

cmd/esc/cli/testdata/env-tag-update-key.yaml

pgavlin · 2024-07-30T00:00:44Z

cmd/esc/cli/testdata/env-tag-mv.yaml

+run: |
+  esc env tag ls test-org/env
+  esc env tag mv test-org/env team esc
+  esc env tag mv test-org/env team owner pulumi


what does this do? does this rename the tag and give it a new value?

pgavlin · 2024-07-30T00:01:31Z

cmd/esc/cli/testdata/env-tag.yaml

+run: |
+  esc env tag ls test-org/env
+  esc env tag test-org/env owner pulumi
+  esc env tag get test-org/env owner


do we need a separate get command? could tag w/o a value just return the current value of the tag?

It's not required here since the return value of tag does that but I added it to the test to demonstrate that it is being persisted

pgavlin · 2024-07-30T00:50:44Z

cmd/esc/cli/env_tag_mv.go

+	var utc bool
+
+	cmd := &cobra.Command{
+		Use:   "mv [<org-name>/]<environment-name> <name> [<newName>] <value>",


I would remove the <value> bit here and make this command purely a "rename" command:

mv [<org-name>/]<environment-name> <name> <newName>

ah ok and keep the value updates under the tag "set" command?

added back the update logic to the implied "set" command

pgavlin · 2024-07-30T00:51:29Z

cmd/esc/cli/env_tag.go

+		Use:   "tag [<org-name>/]<environment-name> <name> <value>",
+		Args:  cobra.ExactArgs(3),


should we push this functionality under a set command?

I think this is good for consistency since revision tags don't explicitly call it set but instead just rely on tag

cmd/esc/cli/env_tag_mv.go

pgavlin · 2024-07-30T05:38:03Z

cmd/esc/cli/cli_test.go

@@ -253,13 +253,14 @@ type testEnvironmentRetract struct {
 type testEnvironmentRevision struct {
 	number    int
 	yaml      []byte
-	tag       string
+	tags      map[string]bool


curious why this changed? this is tracking the revision's etag, which should be static?

Oh interesting - this was used by the test files as a way to pass in a revision tag so I modified it to take multiple tags to mirror production. I did notice it was also being used as the etag but I figured that was solely out of convenience

I did notice it was also being used as the etag but I figured that was solely out of convenience

ah interesting--this double duty was unintentional, and this should be the etag. for convenience you can leave these changes if you like and I can straighten things out later

Co-authored-by: Pat Gavlin <pat@pulumi.com>

pgavlin · 2024-07-30T19:22:43Z

cmd/esc/cli/cli_test.go

@@ -253,13 +253,14 @@ type testEnvironmentRetract struct {
 type testEnvironmentRevision struct {
 	number    int
 	yaml      []byte
-	tag       string
+	tags      map[string]bool


I did notice it was also being used as the etag but I figured that was solely out of convenience

ah interesting--this double duty was unintentional, and this should be the etag. for convenience you can leave these changes if you like and I can straighten things out later

dschaller requested a review from pgavlin June 25, 2024 23:26

dschaller marked this pull request as draft June 25, 2024 23:26

cli: add support for environment tags

fb4c819

dschaller force-pushed the dschaller/envtags branch from 57dd82e to fb4c819 Compare June 25, 2024 23:29

dschaller marked this pull request as ready for review June 25, 2024 23:29

utc over gostring

b0d2dc0

dschaller added 2 commits July 9, 2024 16:04

update commands based on feedback

c9b1176

change tag delimiter

0adc94b

Merge branch 'main' into dschaller/envtags

f8e942f

pgavlin reviewed Jul 11, 2024

View reviewed changes

cmd/esc/cli/env_tag.go Outdated Show resolved Hide resolved

cmd/esc/cli/env_tag.go Outdated Show resolved Hide resolved

dschaller added 2 commits July 17, 2024 11:43

update for new endpoints

043cd27

1.20 compatability

9fa4f3e

pgavlin reviewed Jul 24, 2024

View reviewed changes

generate cli tests for env tags

3be1962

pgavlin requested changes Jul 29, 2024

View reviewed changes

feedback

ede7b8a

pgavlin reviewed Jul 30, 2024

View reviewed changes

dschaller added 5 commits July 29, 2024 17:25

s/tag/tags/g

27a6392

undo generated changes

b3bc337

use utc TS as that's local for GH action

ff36b34

lint

74b1079

update env-tag utc ts

663f752

pgavlin reviewed Jul 30, 2024

View reviewed changes

feedback

1138532

dschaller added 2 commits July 29, 2024 18:25

undo generation

2e20f2a

remove invalid dup test case

1f19a6f

pgavlin reviewed Jul 30, 2024

View reviewed changes

Update cmd/esc/cli/env_tag_mv.go

70ea48f

Co-authored-by: Pat Gavlin <pat@pulumi.com>

pgavlin approved these changes Jul 30, 2024

View reviewed changes

Merge branch 'main' into dschaller/envtags

d6b14b5

dschaller merged commit a9d5522 into main Jul 30, 2024
6 checks passed

dschaller deleted the dschaller/envtags branch July 30, 2024 19:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cli: add support for environment tags #345

cli: add support for environment tags #345

dschaller commented Jun 25, 2024 •

edited

Loading

dschaller commented Jun 26, 2024

pgavlin commented Jun 26, 2024

dschaller commented Jun 27, 2024 •

edited

Loading

dschaller commented Jul 9, 2024

pgavlin commented Jul 19, 2024

dschaller commented Jul 23, 2024

pgavlin Jul 24, 2024

pgavlin Jul 29, 2024

dschaller Jul 29, 2024

pgavlin Jul 29, 2024

dschaller Jul 29, 2024

dschaller Jul 30, 2024

pgavlin Jul 30, 2024

dschaller Jul 30, 2024

pgavlin Jul 30, 2024

dschaller Jul 30, 2024

pgavlin Jul 30, 2024

dschaller Jul 30, 2024

dschaller Jul 30, 2024

pgavlin Jul 30, 2024

dschaller Jul 30, 2024

pgavlin Jul 30, 2024

dschaller Jul 30, 2024 •

edited

Loading

pgavlin Jul 30, 2024

pgavlin Jul 30, 2024

		Use: "tag [<org-name>/]<environment-name> <name> <value>",
		Args: cobra.ExactArgs(3),

cli: add support for environment tags #345

cli: add support for environment tags #345

Conversation

dschaller commented Jun 25, 2024 • edited Loading

dschaller commented Jun 26, 2024

pgavlin commented Jun 26, 2024

dschaller commented Jun 27, 2024 • edited Loading

dschaller commented Jul 9, 2024

pgavlin commented Jul 19, 2024

dschaller commented Jul 23, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dschaller Jul 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dschaller commented Jun 25, 2024 •

edited

Loading

dschaller commented Jun 27, 2024 •

edited

Loading

dschaller Jul 30, 2024 •

edited

Loading