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.

Sign up for GitHub

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

Jump to bottom

fix(cli): configure for non interactive environments #3659

Merged
merged 2 commits into from
Feb 16, 2024
Merged

fix(cli): configure for non interactive environments #3659

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a8f83b5
fix(cli): configure for non interactive environments
schoren Feb 16, 2024
3f8192e
fix
schoren Feb 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
78 changes: 45 additions & 33 deletions cli/config/configurator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ type Configurator struct {
ui cliUI.UI
onFinish onFinishFn
errorHandlerFn errorHandlerFn
flags agentConfig.Flags
flags *agentConfig.Flags
finalServerURL string
}

Expand All @@ -38,7 +38,7 @@ func NewConfigurator(resources *resourcemanager.Registry) Configurator {
errorHandlerFn: func(ctx context.Context, err error) {
ui.Exit(err.Error())
},
flags: agentConfig.Flags{},
flags: &agentConfig.Flags{},
}
}

Expand All @@ -55,12 +55,21 @@ func (c Configurator) WithErrorHandler(fn errorHandlerFn) Configurator {
}

func (c Configurator) Start(ctx context.Context, prev *Config, flags agentConfig.Flags) error {
c.flags = flags
serverURL, err := c.getServerURL(prev, flags)
c.finalServerURL = serverURL
if err != nil {
return err
c.flags = &flags
Copy link
Collaborator Author

@schoren schoren Feb 16, 2024 •
edited

Choose a reason for hiding this comment

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

the original flags value was passed around in functions call, on top of being saved to the instance. this was confusing to follow and created 2 different copies of the same value, so modifications were almost impossible to track

All references to flags where replaced by c.flags and that in turn was converted to a pointer to allow modifications

var serverURL string

// if token is passed, we cannot assume an interactive environment,
// so fallback to last used
if c.flags.Token != "" {
serverURL = lastUsedURL(prev)
} else {
var err error
serverURL, err = c.getServerURL(prev)
if err != nil {
return err
}
}
c.finalServerURL = serverURL

cfg, err := c.createConfig(serverURL)
if err != nil {
Expand All @@ -77,35 +86,38 @@ func (c Configurator) Start(ctx context.Context, prev *Config, flags agentConfig
return nil
}

if flags.CI {
if c.flags.CI {
_, err = Save(ctx, cfg)
if err != nil {
return err
}
return nil
}

_, err = c.handleOAuth(ctx, cfg, prev, flags)
_, err = c.handleOAuth(ctx, cfg, prev)
if err != nil {
return err
}

return nil
}

func (c Configurator) getServerURL(prev *Config, flags agentConfig.Flags) (string, error) {
serverURL := flags.ServerURL
func lastUsedURL(prev *Config) string {
possibleValues := []string{}
if prev != nil {
possibleValues = append(possibleValues, prev.UIEndpoint, prev.URL())
}
possibleValues = append(possibleValues, DefaultCloudEndpoint)

// 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)
return getFirstNonEmptyString(possibleValues)
}

func (c Configurator) getServerURL(prev *Config) (string, error) {
serverURL := c.flags.ServerURL

lastUsed := getFirstNonEmptyString(possibleValues)
serverURL = c.ui.TextInput("What tracetest server do you want to use?", lastUsed)
// if flag was passed, don't show prompt
if c.flags.ServerURL == "" {
serverURL = c.ui.TextInput("What tracetest server do you want to use?", lastUsedURL(prev))
}

if err := validateServerURL(serverURL); err != nil {
Expand Down Expand Up @@ -178,30 +190,30 @@ func (c Configurator) populateConfigWithVersionInfo(ctx context.Context, cfg Con
return cfg, nil, false
}

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

if flags.Token != "" {
if c.flags.Token != "" {
var err error
cfg, err = c.exchangeToken(cfg, flags.Token)
cfg, err = c.exchangeToken(cfg, c.flags.Token)
if err != nil {
return Config{}, err
}
}

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

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

Expand Down Expand Up @@ -262,17 +274,17 @@ func (c Configurator) onOAuthSuccess(ctx context.Context, cfg Config, prev *Conf
cfg.Jwt = jwt
cfg.Token = token

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

func (c Configurator) onOAuthFailure(err error) {
c.errorHandlerFn(context.Background(), err)
}

func (c Configurator) showOrganizationSelector(ctx context.Context, prev *Config, cfg Config, flags agentConfig.Flags) {
cfg.OrganizationID = flags.OrganizationID
if cfg.OrganizationID == "" && flags.AgentApiKey == "" {
func (c Configurator) showOrganizationSelector(ctx context.Context, prev *Config, cfg Config) {
cfg.OrganizationID = c.flags.OrganizationID
if cfg.OrganizationID == "" && c.flags.AgentApiKey == "" {
orgID, err := c.organizationSelector(ctx, cfg, prev)
if err != nil {
c.errorHandlerFn(ctx, err)
Expand All @@ -282,8 +294,8 @@ func (c Configurator) showOrganizationSelector(ctx context.Context, prev *Config
cfg.OrganizationID = orgID
}

cfg.EnvironmentID = flags.EnvironmentID
if cfg.EnvironmentID == "" && flags.AgentApiKey == "" {
cfg.EnvironmentID = c.flags.EnvironmentID
if cfg.EnvironmentID == "" && c.flags.AgentApiKey == "" {
envID, err := c.environmentSelector(ctx, cfg, prev)
if err != nil {
c.errorHandlerFn(ctx, err)
Expand Down
4 changes: 2 additions & 2 deletions cli/config/selector.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@ type Entry struct {
func (c Configurator) organizationSelector(ctx context.Context, cfg Config, prev *Config) (string, error) {
resource, err := c.resources.Get("organization")
if err != nil {
return "", err
return "", fmt.Errorf("cannot create organizations client: %w", err)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

some extra error info helped me debug the issue

}

elements, err := getElements(ctx, resource, cfg)
if err != nil {
return "", err
return "", fmt.Errorf("cannot get organization: %w", err)
}

if len(elements) == 1 {
Expand Down