Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change config semantics to args and full key path #238

Merged
merged 1 commit into from
Aug 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
181 changes: 25 additions & 156 deletions cli/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,129 +24,28 @@ package cli

import (
"fmt"
"strings"

"github.com/urfave/cli/v2"

"github.com/temporalio/tctl-kit/pkg/color"
"github.com/temporalio/tctl-kit/pkg/config"
)

var (
rootKeys = []string{
config.KeyActive,
"version",
}
envKeys = []string{
FlagNamespace,
FlagAddress,
FlagTLSCertPath,
FlagTLSKeyPath,
FlagTLSCaPath,
FlagTLSDisableHostVerification,
FlagTLSServerName,
}
keys = append(rootKeys, envKeys...)
)

func newConfigCommands() []*cli.Command {
return []*cli.Command{
{
Name: "get",
Usage: "Print config values",
Flags: []cli.Flag{
&cli.BoolFlag{
Name: config.KeyActive,
Usage: "Print active environment",
},
&cli.BoolFlag{
Name: config.KeyAlias,
Usage: "Print command aliases",
},
&cli.BoolFlag{
Name: FlagNamespace,
Usage: "Print default namespace (for active environment)",
},
&cli.BoolFlag{
Name: FlagAddress,
Usage: "Print Temporal address (for active environment)",
},
&cli.StringFlag{
Name: FlagTLSCertPath,
Value: "",
Usage: "Print path to x509 certificate",
},
&cli.StringFlag{
Name: FlagTLSKeyPath,
Value: "",
Usage: "Print path to private key",
},
&cli.StringFlag{
Name: FlagTLSCaPath,
Value: "",
Usage: "Print path to server CA certificate",
},
&cli.BoolFlag{
Name: FlagTLSDisableHostVerification,
Usage: "Print whether tls host name verification is disabled",
},
&cli.StringFlag{
Name: FlagTLSServerName,
Value: "",
Usage: "Print target TLS server name",
},
},
Flags: []cli.Flag{},
Action: func(c *cli.Context) error {
return GetValue(c)
},
},
{
Name: "set",
Usage: "Set config values",
Flags: []cli.Flag{
&cli.StringFlag{
Name: config.KeyActive,
Usage: "Activate environment",
Value: "local",
},
&cli.StringFlag{
Name: config.KeyAlias,
Usage: "Create an alias command",
},
&cli.StringFlag{
Name: "version",
Usage: "Opt-in to a new TCTL UX, values: (current, next)",
},
&cli.StringFlag{
Name: FlagAddress,
Usage: "host:port for Temporal frontend service",
Value: localHostPort,
},
&cli.StringFlag{
Name: FlagTLSCertPath,
Value: "",
Usage: "Path to x509 certificate",
},
&cli.StringFlag{
Name: FlagTLSKeyPath,
Value: "",
Usage: "Path to private key",
},
&cli.StringFlag{
Name: FlagTLSCaPath,
Value: "",
Usage: "Path to server CA certificate",
},
&cli.BoolFlag{
Name: FlagTLSDisableHostVerification,
Usage: "Disable TLS host name verification (TLS must be enabled)",
},
&cli.StringFlag{
Name: FlagTLSServerName,
Value: "",
Usage: "Override for target server name",
},
},
Flags: []cli.Flag{},

Action: func(c *cli.Context) error {
return SetValue(c)
},
Expand All @@ -155,50 +54,38 @@ func newConfigCommands() []*cli.Command {
}

func GetValue(c *cli.Context) error {
for _, key := range keys {
if !c.IsSet(key) {
continue
}
for i := 0; i < c.Args().Len(); i++ {
key := c.Args().Get(i)
val, err := tctlConfig.Get(key)

var val interface{}
var err error
if isRootKey(key) {
val, err = tctlConfig.Get(c, key)
} else if isEnvKey(key) {
val, err = tctlConfig.GetByEnvironment(c, key)
} else {
return fmt.Errorf("invalid key: %s", key)
}
if err != nil {
return err
}

fmt.Printf("%v: %v\n", color.Magenta(c, "%v", key), val)
}

return nil
}

func SetValue(c *cli.Context) error {
for _, key := range keys {
if !c.IsSet(key) {
continue
}
if c.Args().Len() == 0 {
return fmt.Errorf("no key specified")
}

val := c.String(key)
if isRootKey(key) {
if err := tctlConfig.Set(c, key, val); err != nil {
return fmt.Errorf("unable to set property %s: %s", key, err)
}
} else if isEnvKey(key) {
if err := tctlConfig.SetByEnvironment(c, key, val); err != nil {
return fmt.Errorf("unable to set property %s: %s", key, err)
}
} else {
return fmt.Errorf("unable to set property %s: invalid key", key)
}
fmt.Printf("%v: %v\n", color.Magenta(c, "%v", key), val)
key := c.Args().Get(0)

var val string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When is it valid to omit the value?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was thinking of booleans such as tls-disable-host-verification

Copy link
Contributor Author

@feedmeapples feedmeapples Aug 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so tctl config set tls-disable-host-verification would mean it being set as true. That's similar to when it is passed as a flag

Copy link
Member

@bergundy bergundy Aug 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm.. how do we deal with booleans and numbers in general?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

made them to work with the standard urfave/cli syntax #257

if c.Args().Len() > 1 {
val = c.Args().Get(1)
}

if err := tctlConfig.Set(key, val); err != nil {
return fmt.Errorf("unable to set property %s: %s", key, err)
}

fmt.Printf("%v: %v\n", color.Magenta(c, "%v", key), val)

return nil
}

Expand Down Expand Up @@ -226,34 +113,16 @@ func newAliasCommand() *cli.Command {
}
}

func isRootKey(key string) bool {
for _, k := range rootKeys {
if strings.Compare(key, k) == 0 {
return true
}
}
return false
}

func isEnvKey(key string) bool {
for _, k := range envKeys {
if strings.Compare(key, k) == 0 {
return true
}
}
return false
}

func createAlias(c *cli.Context) error {
command := c.String("command")
alias := c.String("alias")

fullKey := fmt.Sprintf("%s.%s", config.KeyAlias, command)
fullKey := fmt.Sprintf("%s.%s", config.KeyAliases, command)

if err := tctlConfig.Set(c, fullKey, alias); err != nil {
return fmt.Errorf("unable to set property %s: %s", config.KeyAlias, err)
if err := tctlConfig.Set(fullKey, alias); err != nil {
return fmt.Errorf("unable to set property %s: %s", config.KeyAliases, err)
}

fmt.Printf("%v: %v\n", color.Magenta(c, "%v", config.KeyAlias), alias)
fmt.Printf("%v: %v\n", color.Magenta(c, "%v", config.KeyAliases), alias)
return nil
}
2 changes: 1 addition & 1 deletion cli/schedule_commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ func BackfillSchedule(c *cli.Context) error {
ScheduleId: scheduleID,
Patch: &schedpb.SchedulePatch{
BackfillRequest: []*schedpb.BackfillRequest{
&schedpb.BackfillRequest{
{
StartTime: timestamp.TimePtr(startTime),
EndTime: timestamp.TimePtr(endTime),
OverlapPolicy: overlap,
Expand Down
9 changes: 2 additions & 7 deletions cli/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ func getCurrentUserFromEnv() string {
return os.Getenv(n)
}
}
return "unkown"
return "unknown"
}

func prettyPrintJSONObject(o interface{}) {
Expand Down Expand Up @@ -378,12 +378,7 @@ func readFlagOrConfig(c *cli.Context, key string) string {
return c.String(key)
}

var cVal string
if isRootKey(key) {
cVal, _ = tctlConfig.Get(c, key)
} else if isEnvKey(key) {
cVal, _ = tctlConfig.GetByEnvironment(c, key)
}
cVal, _ := tctlConfig.GetByCurrentEnvironment(key)

if cVal != "" {
return cVal
Expand Down
4 changes: 2 additions & 2 deletions cli_curr/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func GetValue(c *cli.Context) error {
return fmt.Errorf("unable to get property %v. %s", key, err)
}

val, err := tctlConfig.Get(nil, key)
val, err := tctlConfig.Get(key)
if err != nil {
return fmt.Errorf("unable to get property %v. %s", key, err)
}
Expand All @@ -90,7 +90,7 @@ func SetValue(c *cli.Context) error {
return fmt.Errorf("unable to set property %v. %s", key, err)
}

if err := tctlConfig.Set(nil, key, val); err != nil {
if err := tctlConfig.Set(key, val); err != nil {
return fmt.Errorf("unable to set property %v. %s", key, err)
}

Expand Down
2 changes: 1 addition & 1 deletion cmd/tctl/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import (
// See https://docs.temporal.io/docs/system-tools/tctl/ for usage
func main() {
tctlConfig, _ := config.NewTctlConfig()
version, _ := tctlConfig.Get(nil, "version")
version, _ := tctlConfig.Get("version")

if version == "next" {
appNext := cli.NewCliApp()
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ require (
github.com/olivere/elastic/v7 v7.0.32
github.com/pborman/uuid v1.2.1
github.com/stretchr/testify v1.8.0
github.com/temporalio/tctl-kit v0.0.0-20220512165751-9c751176dd14
github.com/temporalio/tctl-kit v0.0.0-20220810223931-23f1dcdbad08
github.com/urfave/cli v1.22.9
github.com/urfave/cli/v2 v2.10.2
go.temporal.io/api v1.10.0
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,8 @@ github.com/temporalio/ringpop-go v0.0.0-20211012191444-6f91b5915e95 h1:8G34e73qt
github.com/temporalio/ringpop-go v0.0.0-20211012191444-6f91b5915e95/go.mod h1:Ek9J8CAfI1IwVSqHpTOgj7FjzRSJ5SM/ud52eCmkhsw=
github.com/temporalio/tctl-kit v0.0.0-20220512165751-9c751176dd14 h1:4qVgd+wa3QAJL+CAZbxJUaOaygOeRXgQTNGJ10cUp5E=
github.com/temporalio/tctl-kit v0.0.0-20220512165751-9c751176dd14/go.mod h1:2Ef/g3r5xjrvCe36AX4SXS0IUhD1plwwexNyVn8TB4I=
github.com/temporalio/tctl-kit v0.0.0-20220810223931-23f1dcdbad08 h1:PMXGN2MzWUFCdaVm7K38spW7NNKRwUNyPNjb2OBlAu8=
github.com/temporalio/tctl-kit v0.0.0-20220810223931-23f1dcdbad08/go.mod h1:2Ef/g3r5xjrvCe36AX4SXS0IUhD1plwwexNyVn8TB4I=
github.com/twmb/murmur3 v1.1.5/go.mod h1:Qq/R7NUyOfr65zD+6Q5IHKsJLwP7exErjN6lyyq3OSQ=
github.com/twmb/murmur3 v1.1.6 h1:mqrRot1BRxm+Yct+vavLMou2/iJt0tNVTTC0QoIjaZg=
github.com/twmb/murmur3 v1.1.6/go.mod h1:Qq/R7NUyOfr65zD+6Q5IHKsJLwP7exErjN6lyyq3OSQ=
Expand Down