From 69af57cfb49b26e239a938ed67508f4bc2ab0c64 Mon Sep 17 00:00:00 2001 From: Spencer Judge Date: Tue, 28 May 2024 11:20:14 -0700 Subject: [PATCH] Fail on unknown environments (#578) --- temporalcli/commands.env.go | 18 ++++++------------ temporalcli/commands.env_test.go | 12 ++++++++++-- temporalcli/commands.gen.go | 4 ++++ temporalcli/commands.go | 11 +++++++++++ temporalcli/commands_test.go | 6 +++++- temporalcli/commandsmd/code.go | 4 ++++ temporalcli/commandsmd/commands.md | 5 +++++ temporalcli/commandsmd/parse.go | 25 ++++++++++++++----------- 8 files changed, 59 insertions(+), 26 deletions(-) diff --git a/temporalcli/commands.env.go b/temporalcli/commands.env.go index 8433b884..6d536398 100644 --- a/temporalcli/commands.env.go +++ b/temporalcli/commands.env.go @@ -8,11 +8,11 @@ import ( "github.com/temporalio/cli/temporalcli/internal/printer" ) -func (c *TemporalEnvCommand) envNameAndKey(cctx *CommandContext, args[] string, keyFlag string) (string, string, error) { +func (c *TemporalEnvCommand) envNameAndKey(cctx *CommandContext, args []string, keyFlag string) (string, string, error) { if len(args) > 0 { cctx.Logger.Warn("Arguments to env commands are deprecated; please use --env and --key (or -k) instead") - if (c.Parent.Env != "default" || keyFlag != "") { + if c.Parent.Env != "default" || keyFlag != "" { return "", "", fmt.Errorf("cannot specify both an argument and flags; please use flags instead") } @@ -42,11 +42,8 @@ func (c *TemporalEnvDeleteCommand) run(cctx *CommandContext, args []string) erro return err } - // Env must be present - env, ok := cctx.EnvConfigValues[envName] - if !ok { - return fmt.Errorf("env %q not found", envName) - } + // Env is guaranteed to already be present + env, _ := cctx.EnvConfigValues[envName] // User can remove single flag or all in env if key != "" { cctx.Logger.Info("Deleting env property", "env", envName, "property", key) @@ -64,11 +61,8 @@ func (c *TemporalEnvGetCommand) run(cctx *CommandContext, args []string) error { return err } - // Env must be present - env, ok := cctx.EnvConfigValues[envName] - if !ok { - return fmt.Errorf("env %q not found", envName) - } + // Env is guaranteed to already be present + env, _ := cctx.EnvConfigValues[envName] type prop struct { Property string `json:"property"` Value string `json:"value"` diff --git a/temporalcli/commands.env_test.go b/temporalcli/commands.env_test.go index 32720af0..0956485e 100644 --- a/temporalcli/commands.env_test.go +++ b/temporalcli/commands.env_test.go @@ -14,7 +14,7 @@ func TestEnv_Simple(t *testing.T) { // Non-existent file, no env found for get h.Options.EnvConfigFile = "does-not-exist" res := h.Execute("env", "get", "--env", "myenv1") - h.ErrorContains(res.Err, `env "myenv1" not found`) + h.ErrorContains(res.Err, `environment "myenv1" not found`) // Temp file for env tmpFile, err := os.CreateTemp("", "") @@ -83,7 +83,15 @@ func TestEnv_InputValidation(t *testing.T) { h := NewCommandHarness(t) defer h.Close() - res := h.Execute("env", "get", "--env", "myenv1", "foo.bar") + // myenv1 needs to exist + tmpFile, err := os.CreateTemp("", "") + h.NoError(err) + h.Options.EnvConfigFile = tmpFile.Name() + defer os.Remove(h.Options.EnvConfigFile) + res := h.Execute("env", "set", "--env", "myenv1", "-k", "foo", "-v", "bar") + h.NoError(res.Err) + + res = h.Execute("env", "get", "--env", "myenv1", "foo.bar") h.ErrorContains(res.Err, `cannot specify both`) res = h.Execute("env", "get", "-k", "key", "foo.bar") diff --git a/temporalcli/commands.gen.go b/temporalcli/commands.gen.go index eeabddf8..86732617 100644 --- a/temporalcli/commands.gen.go +++ b/temporalcli/commands.gen.go @@ -341,6 +341,8 @@ func NewTemporalEnvListCommand(cctx *CommandContext, parent *TemporalEnvCommand) s.Command.Short = "Print all environments." s.Command.Long = "List all environments." s.Command.Args = cobra.NoArgs + s.Command.Annotations = make(map[string]string) + s.Command.Annotations["ignoresMissingEnv"] = "true" s.Command.Run = func(c *cobra.Command, args []string) { if err := s.run(cctx, args); err != nil { cctx.Options.Fail(err) @@ -368,6 +370,8 @@ func NewTemporalEnvSetCommand(cctx *CommandContext, parent *TemporalEnvCommand) s.Command.Long = "`temporal env set --env environment -k property -v value`\n\nProperty names match CLI option names, for example '--address' and '--tls-cert-path':\n\n`temporal env set --env prod -k address -v 127.0.0.1:7233`\n`temporal env set --env prod -k tls-cert-path -v /home/my-user/certs/cluster.cert`\n\nIf the environment is not specified, the `default` environment is used." } s.Command.Args = cobra.MaximumNArgs(2) + s.Command.Annotations = make(map[string]string) + s.Command.Annotations["ignoresMissingEnv"] = "true" s.Command.Flags().StringVarP(&s.Key, "key", "k", "", "The name of the property.") s.Command.Flags().StringVarP(&s.Value, "value", "v", "", "The value to set the property to.") s.Command.Run = func(c *cobra.Command, args []string) { diff --git a/temporalcli/commands.go b/temporalcli/commands.go index 97ad694f..bc039a65 100644 --- a/temporalcli/commands.go +++ b/temporalcli/commands.go @@ -376,6 +376,17 @@ func (c *TemporalCommand) initCommand(cctx *CommandContext) { color.NoColor = true } cctx.ActuallyRanCommand = true + + if cctx.Options.EnvConfigName != "default" { + if _, ok := cctx.EnvConfigValues[cctx.Options.EnvConfigName]; !ok { + if _, ok := cmd.Annotations["ignoresMissingEnv"]; !ok { + // stfu about help output + cmd.SilenceErrors = true + cmd.SilenceUsage = true + return fmt.Errorf("environment %q not found", cctx.Options.EnvConfigName) + } + } + } return res } c.Command.PersistentPostRun = func(*cobra.Command, []string) { diff --git a/temporalcli/commands_test.go b/temporalcli/commands_test.go index 839e6d7a..2f7bafe5 100644 --- a/temporalcli/commands_test.go +++ b/temporalcli/commands_test.go @@ -134,10 +134,14 @@ func (h *CommandHarness) Execute(args ...string) *CommandResult { options.Args = args // Disable env if no env file and no --env-file arg options.DisableEnvConfig = options.EnvConfigFile == "" && !slices.Contains(args, "--env-file") + // Set default env name if disabled, otherwise we'll fail with missing environment + if options.DisableEnvConfig { + options.EnvConfigName = "default" + } // Capture error options.Fail = func(err error) { if res.Err != nil { - panic("fail called twice") + panic("fail called twice, just failed with " + err.Error()) } res.Err = err } diff --git a/temporalcli/commandsmd/code.go b/temporalcli/commandsmd/code.go index f33d9558..ea62c498 100644 --- a/temporalcli/commandsmd/code.go +++ b/temporalcli/commandsmd/code.go @@ -194,6 +194,10 @@ func (c *Command) writeCode(w *codeWriter) error { } else { w.writeLinef("s.Command.Args = %v.NoArgs", w.importCobra()) } + if c.IgnoreMissingEnv { + w.writeLinef("s.Command.Annotations = make(map[string]string)") + w.writeLinef("s.Command.Annotations[\"ignoresMissingEnv\"] = \"true\"") + } // Add subcommands for _, subCommand := range subCommands { w.writeLinef("s.Command.AddCommand(&New%v(cctx, &s).Command)", subCommand.structName()) diff --git a/temporalcli/commandsmd/commands.md b/temporalcli/commandsmd/commands.md index 0f5db1eb..a4a02241 100644 --- a/temporalcli/commandsmd/commands.md +++ b/temporalcli/commandsmd/commands.md @@ -201,6 +201,10 @@ If the environment is not specified, the `default` environment is used. List all environments. + + ### temporal env set: Set environment properties. `temporal env set --env environment -k property -v value` @@ -214,6 +218,7 @@ If the environment is not specified, the `default` environment is used. #### Options diff --git a/temporalcli/commandsmd/parse.go b/temporalcli/commandsmd/parse.go index b11c4ff5..1211f617 100644 --- a/temporalcli/commandsmd/parse.go +++ b/temporalcli/commandsmd/parse.go @@ -15,17 +15,18 @@ import ( var CommandsMarkdown []byte type Command struct { - FullName string - NamePath []string - UseSuffix string - Short string - LongPlain string - LongHighlighted string - LongMarkdown string - OptionsSets []CommandOptions - HasInit bool - ExactArgs int - MaximumArgs int + FullName string + NamePath []string + UseSuffix string + Short string + LongPlain string + LongHighlighted string + LongMarkdown string + OptionsSets []CommandOptions + HasInit bool + ExactArgs int + MaximumArgs int + IgnoreMissingEnv bool } type CommandOptions struct { @@ -117,6 +118,8 @@ func (c *Command) parseSection(section string) error { if c.MaximumArgs, err = strconv.Atoi(strings.TrimPrefix(bullet, "* maximum-args=")); err != nil { return fmt.Errorf("invalid maximum-args: %w", err) } + case strings.HasPrefix(bullet, "* ignores-missing-env"): + c.IgnoreMissingEnv = true default: return fmt.Errorf("unrecognized attribute bullet: %q", bullet) }