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

feat(cli): improve configure cmd ux #3588

Merged
merged 2 commits into from
Feb 16, 2024
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
7 changes: 6 additions & 1 deletion cli/cmd/configure_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ var configureCmd = &cobra.Command{
CI: configParams.CI,
}

config, err := config.LoadConfig("")
if err != nil {
return "", err
}

if flagProvided(cmd, "server-url") || flagProvided(cmd, "endpoint") {
flags.ServerURL = configParams.ServerURL
}
Expand All @@ -43,7 +48,7 @@ var configureCmd = &cobra.Command{
flags.OrganizationID = configParams.OrganizationID
}

return "", configurator.Start(ctx, nil, flags)
return "", configurator.Start(ctx, &config, flags)
})),
PostRun: teardownCommand,
}
Expand Down
2 changes: 1 addition & 1 deletion cli/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func loadConfig(configFile string) (Config, error) {
return config, nil
}

func ValidateServerURL(serverURL string) error {
func validateServerURL(serverURL string) error {
if !strings.HasPrefix(serverURL, "http://") && !strings.HasPrefix(serverURL, "https://") {
return fmt.Errorf(`the server URL must start with the scheme, either "http://" or "https://"`)
}
Expand Down
47 changes: 27 additions & 20 deletions cli/config/configurator.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,16 +83,21 @@ func (c Configurator) Start(ctx context.Context, prev *Config, flags agentConfig
}

func (c Configurator) getServerURL(prev *Config, flags agentConfig.Flags) (string, error) {
var prevUIEndpoint string
if prev != nil {
prevUIEndpoint = prev.UIEndpoint
}
serverURL := getFirstValidString(flags.ServerURL, prevUIEndpoint)
if serverURL == "" {
serverURL = c.ui.TextInput("What tracetest server do you want to use?", DefaultCloudEndpoint)
serverURL := flags.ServerURL

// if flag was passed, don't show prompt
if flags.ServerURL == "" {
possibleValues := []string{}
if prev != nil {
possibleValues = append(possibleValues, prev.UIEndpoint, prev.URL())
}
possibleValues = append(possibleValues, DefaultCloudEndpoint)

lastUsed := getFirstNonEmptyString(possibleValues)
serverURL = c.ui.TextInput("What tracetest server do you want to use?", lastUsed)
}

if err := ValidateServerURL(serverURL); err != nil {
if err := validateServerURL(serverURL); err != nil {
return "", err
}

Expand Down Expand Up @@ -163,9 +168,11 @@ func (c Configurator) populateConfigWithVersionInfo(ctx context.Context, cfg Con
}

func (c Configurator) handleOAuth(ctx context.Context, cfg Config, prev *Config, flags agentConfig.Flags) (Config, error) {
if prev != nil && prev.Jwt != "" {
cfg.Jwt = prev.Jwt
cfg.Token = prev.Token
if prev != nil && cfg.URL() == prev.URL() {
if prev != nil && prev.Jwt != "" {
cfg.Jwt = prev.Jwt
cfg.Token = prev.Token
}
}

if flags.Token != "" {
Expand All @@ -178,17 +185,17 @@ func (c Configurator) handleOAuth(ctx context.Context, cfg Config, prev *Config,

if flags.AgentApiKey != "" {
cfg.AgentApiKey = flags.AgentApiKey
c.ShowOrganizationSelector(ctx, cfg, flags)
c.showOrganizationSelector(ctx, prev, cfg, flags)
return cfg, nil
}

if cfg.Jwt != "" {
c.ShowOrganizationSelector(ctx, cfg, flags)
c.showOrganizationSelector(ctx, prev, cfg, flags)
return cfg, nil
}

oauthServer := oauth.NewOAuthServer(cfg.OAuthEndpoint(), cfg.UIEndpoint)
err := oauthServer.WithOnSuccess(c.onOAuthSuccess(ctx, cfg)).
err := oauthServer.WithOnSuccess(c.onOAuthSuccess(ctx, cfg, prev)).
WithOnFailure(c.onOAuthFailure).
GetAuthJWT()
if err != nil {
Expand Down Expand Up @@ -225,7 +232,7 @@ func (c Configurator) exchangeToken(cfg Config, token string) (Config, error) {
return cfg, nil
}

func getFirstValidString(values ...string) string {
func getFirstNonEmptyString(values []string) string {
for _, v := range values {
if v != "" {
return v
Expand All @@ -235,23 +242,23 @@ func getFirstValidString(values ...string) string {
return ""
}

func (c Configurator) onOAuthSuccess(ctx context.Context, cfg Config) func(token, jwt string) {
func (c Configurator) onOAuthSuccess(ctx context.Context, cfg Config, prev *Config) func(token, jwt string) {
return func(token, jwt string) {
cfg.Jwt = jwt
cfg.Token = token

c.ShowOrganizationSelector(ctx, cfg, c.flags)
c.showOrganizationSelector(ctx, prev, cfg, c.flags)
}
}

func (c Configurator) onOAuthFailure(err error) {
c.ui.Exit(err.Error())
}

func (c Configurator) ShowOrganizationSelector(ctx context.Context, cfg Config, flags agentConfig.Flags) {
func (c Configurator) showOrganizationSelector(ctx context.Context, prev *Config, cfg Config, flags agentConfig.Flags) {
cfg.OrganizationID = flags.OrganizationID
if cfg.OrganizationID == "" && flags.AgentApiKey == "" {
orgID, err := c.organizationSelector(ctx, cfg)
orgID, err := c.organizationSelector(ctx, cfg, prev)
if err != nil {
c.ui.Exit(err.Error())
return
Expand All @@ -262,7 +269,7 @@ func (c Configurator) ShowOrganizationSelector(ctx context.Context, cfg Config,

cfg.EnvironmentID = flags.EnvironmentID
if cfg.EnvironmentID == "" && flags.AgentApiKey == "" {
envID, err := c.environmentSelector(ctx, cfg)
envID, err := c.environmentSelector(ctx, cfg, prev)
if err != nil {
c.ui.Exit(err.Error())
return
Expand Down
21 changes: 16 additions & 5 deletions cli/config/selector.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ type Entry struct {
Name string `json:"name"`
}

func (c Configurator) organizationSelector(ctx context.Context, cfg Config) (string, error) {
func (c Configurator) organizationSelector(ctx context.Context, cfg Config, prev *Config) (string, error) {
resource, err := c.resources.Get("organization")
if err != nil {
return "", err
Expand All @@ -33,6 +33,8 @@ Defaulting to only available Organization: %s`, elements[0].Name))

orgID := ""
options := make([]cliUI.Option, len(elements))

defaultIndex := 0
for i, org := range elements {
options[i] = cliUI.Option{
Text: fmt.Sprintf("%s (%s)", org.Name, org.ID),
Expand All @@ -42,16 +44,20 @@ Defaulting to only available Organization: %s`, elements[0].Name))
}
}(org),
}

// if we have a previous organization, set it as default
if prev != nil && prev.OrganizationID == org.ID {
defaultIndex = i
}
}

option := c.ui.Select(`
What Organization do you want to use?`, options, 0)
option := c.ui.Select(`What Organization do you want to use?`, options, defaultIndex)
option.Fn(c.ui)

return orgID, nil
}

func (c Configurator) environmentSelector(ctx context.Context, cfg Config) (string, error) {
func (c Configurator) environmentSelector(ctx context.Context, cfg Config, prev *Config) (string, error) {
resource, err := c.resources.Get("env")
if err != nil {
return "", err
Expand All @@ -72,6 +78,7 @@ func (c Configurator) environmentSelector(ctx context.Context, cfg Config) (stri

envID := ""
options := make([]cliUI.Option, len(elements))
defaultIndex := 0
for i, env := range elements {
options[i] = cliUI.Option{
Text: fmt.Sprintf("%s (%s)", env.Name, env.ID),
Expand All @@ -81,9 +88,13 @@ func (c Configurator) environmentSelector(ctx context.Context, cfg Config) (stri
}
}(env),
}
// if we have a previous env, set it as default
if prev != nil && prev.EnvironmentID == env.ID {
defaultIndex = i
}
}

option := c.ui.Select("What Environment do you want to use?", options, 0)
option := c.ui.Select("What Environment do you want to use?", options, defaultIndex)
option.Fn(c.ui)

return envID, err
Expand Down
Loading