From dfc6f00982afc17e73e68b01bfbfa811cda88561 Mon Sep 17 00:00:00 2001 From: Sam Arnold Date: Mon, 20 Sep 2021 13:57:45 -0400 Subject: [PATCH] fix: less confusing overloads of 'token' in help (#272) * chore: refactor GetOrg calls * fix: less confusing overloads of 'token' in help Closes #20619 * fix: clean up iscommon function definition --- clients/apply/apply.go | 20 ++------ clients/auth/auth.go | 19 +------ clients/cli.go | 33 ++++++++++++ clients/secret/secret.go | 19 +------ clients/secret/secret_test.go | 6 +-- clients/stacks/stacks.go | 51 ++++--------------- clients/telegrafs/telegrafs.go | 17 +------ clients/template/template.go | 40 +++------------ clients/user/user.go | 26 +--------- clients/user/user_test.go | 2 +- clients/v1_auth/v1_auth.go | 31 +++--------- cmd/influx/global.go | 91 ++++++++++++++++++++++++++-------- 12 files changed, 137 insertions(+), 218 deletions(-) diff --git a/clients/apply/apply.go b/clients/apply/apply.go index cf15181..e9cd439 100644 --- a/clients/apply/apply.go +++ b/clients/apply/apply.go @@ -46,23 +46,9 @@ type ResourceFilter struct { } func (c Client) Apply(ctx context.Context, params *Params) error { - if params.OrgId == "" && params.OrgName == "" && c.ActiveConfig.Org == "" { - return clients.ErrMustSpecifyOrg - } - orgID := params.OrgId - if orgID == "" { - orgName := params.OrgName - if orgName == "" { - orgName = c.ActiveConfig.Org - } - orgs, err := c.GetOrgs(ctx).Org(orgName).Execute() - if err != nil { - return fmt.Errorf("failed to get ID for org %q: %w", orgName, err) - } - if len(orgs.GetOrgs()) == 0 { - return fmt.Errorf("no orgs found with name %q: %w", orgName, err) - } - orgID = orgs.GetOrgs()[0].GetId() + orgID, err := c.GetOrgId(ctx, params.OrgId, params.OrgName, c.OrganizationsApi) + if err != nil { + return err } templates, err := template.ReadSources(ctx, params.Sources) diff --git a/clients/auth/auth.go b/clients/auth/auth.go index 29c315f..c9b8d23 100644 --- a/clients/auth/auth.go +++ b/clients/auth/auth.go @@ -424,22 +424,5 @@ func makePermResource(permType string, id string, orgId string) api.PermissionRe } func (c Client) getOrgID(ctx context.Context, params clients.OrgParams) (string, error) { - if !params.OrgID.Valid() && params.OrgName == "" && c.ActiveConfig.Org == "" { - return "", clients.ErrMustSpecifyOrg - } - if params.OrgID.Valid() { - return params.OrgID.String(), nil - } - name := params.OrgName - if name == "" { - name = c.ActiveConfig.Org - } - org, err := c.GetOrgs(ctx).Org(name).Execute() - if err != nil { - return "", fmt.Errorf("failed to lookup org with name %q: %w", name, err) - } - if len(org.GetOrgs()) == 0 { - return "", fmt.Errorf("no organization with name %q: %w", name, err) - } - return org.GetOrgs()[0].GetId(), nil + return c.GetOrgIdI(ctx, params.OrgID, params.OrgName, c.OrganizationsApi) } diff --git a/clients/cli.go b/clients/cli.go index ed4a24b..14318b4 100644 --- a/clients/cli.go +++ b/clients/cli.go @@ -1,10 +1,14 @@ package clients import ( + "context" "encoding/json" + "fmt" + "github.com/influxdata/influx-cli/v2/api" "github.com/influxdata/influx-cli/v2/config" "github.com/influxdata/influx-cli/v2/internal/tabwriter" + "github.com/influxdata/influx-cli/v2/pkg/influxid" "github.com/influxdata/influx-cli/v2/pkg/stdio" ) @@ -38,3 +42,32 @@ func (c *CLI) PrintTable(headers []string, rows ...map[string]interface{}) error } return nil } + +func (c *CLI) GetOrgId(ctx context.Context, paramOrgId, paramOrgName string, orgApi api.OrganizationsApi) (string, error) { + if paramOrgId != "" { + return paramOrgId, nil + } + orgName := paramOrgName + if orgName == "" { + orgName = c.ActiveConfig.Org + } + if orgName == "" { + return "", ErrMustSpecifyOrg + } + res, err := orgApi.GetOrgs(ctx).Org(orgName).Execute() + if err != nil { + return "", fmt.Errorf("failed to lookup org with name %q: %w", orgName, err) + } + if len(res.GetOrgs()) == 0 { + return "", fmt.Errorf("no organization with name %q", orgName) + } + return res.GetOrgs()[0].GetId(), nil +} + +func (c *CLI) GetOrgIdI(ctx context.Context, paramOrgId influxid.ID, paramOrgName string, orgApi api.OrganizationsApi) (string, error) { + orgId := "" + if paramOrgId.Valid() { + orgId = paramOrgId.String() + } + return c.GetOrgId(ctx, orgId, paramOrgName, orgApi) +} diff --git a/clients/secret/secret.go b/clients/secret/secret.go index 5ab5e9c..c444df9 100644 --- a/clients/secret/secret.go +++ b/clients/secret/secret.go @@ -151,22 +151,5 @@ func (c Client) printSecrets(opts secretPrintOpt) error { } func (c Client) getOrgID(ctx context.Context, params clients.OrgParams) (string, error) { - if params.OrgID.Valid() || params.OrgName != "" || c.ActiveConfig.Org != "" { - if params.OrgID.Valid() { - return params.OrgID.String(), nil - } - for _, name := range []string{params.OrgName, c.ActiveConfig.Org} { - if name != "" { - org, err := c.GetOrgs(ctx).Org(name).Execute() - if err != nil { - return "", fmt.Errorf("failed to lookup org with name %q: %w", name, err) - } - if len(org.GetOrgs()) == 0 { - return "", fmt.Errorf("no organization with name %q: %w", name, err) - } - return org.GetOrgs()[0].GetId(), nil - } - } - } - return "", fmt.Errorf("org or org-id must be provided") + return c.GetOrgIdI(ctx, params.OrgID, params.OrgName, c.OrganizationsApi) } diff --git a/clients/secret/secret_test.go b/clients/secret/secret_test.go index c0085e1..4734b7a 100644 --- a/clients/secret/secret_test.go +++ b/clients/secret/secret_test.go @@ -76,7 +76,7 @@ func TestSecret_List(t *testing.T) { params: secret.ListParams{ OrgParams: clients.OrgParams{}, }, - expectError: "org or org-id must be provided", + expectError: "must specify org ID or org name", }, } @@ -175,7 +175,7 @@ func TestSecret_Delete(t *testing.T) { params: secret.DeleteParams{ Key: fakeKey, }, - expectError: "org or org-id must be provided", + expectError: "must specify org ID or org name", }, } @@ -294,7 +294,7 @@ func TestSecret_Update(t *testing.T) { Key: fakeKey, Value: fakeValue, }, - expectError: "org or org-id must be provided", + expectError: "must specify org ID or org name", }, } diff --git a/clients/stacks/stacks.go b/clients/stacks/stacks.go index 6bb83a4..6645f87 100644 --- a/clients/stacks/stacks.go +++ b/clients/stacks/stacks.go @@ -30,20 +30,9 @@ func (c Client) List(ctx context.Context, params *ListParams) error { return clients.ErrMustSpecifyOrg } - orgId := params.OrgId - if orgId == "" { - orgName := params.OrgName - if orgName == "" { - orgName = c.ActiveConfig.Org - } - res, err := c.GetOrgs(ctx).Org(orgName).Execute() - if err != nil { - return fmt.Errorf("failed to lookup org with name %q: %w", orgName, err) - } - if len(res.GetOrgs()) == 0 { - return fmt.Errorf("no organization with name %q: %w", orgName, err) - } - orgId = res.GetOrgs()[0].GetId() + orgId, err := c.GetOrgId(ctx, params.OrgId, params.OrgName, c.OrganizationsApi) + if err != nil { + return err } res, err := c.ListStacks(ctx).OrgID(orgId).Name(params.StackNames).StackID(params.StackIds).Execute() @@ -68,20 +57,9 @@ func (c Client) Init(ctx context.Context, params *InitParams) error { return clients.ErrMustSpecifyOrg } - orgId := params.OrgId - if orgId == "" { - orgName := params.OrgName - if orgName == "" { - orgName = c.ActiveConfig.Org - } - res, err := c.GetOrgs(ctx).Org(orgName).Execute() - if err != nil { - return fmt.Errorf("failed to lookup org with name %q: %w", orgName, err) - } - if len(res.GetOrgs()) == 0 { - return fmt.Errorf("no organization with name %q: %w", orgName, err) - } - orgId = res.GetOrgs()[0].GetId() + orgId, err := c.GetOrgId(ctx, params.OrgId, params.OrgName, c.OrganizationsApi) + if err != nil { + return err } req := api.StackPostRequest{ @@ -114,20 +92,9 @@ func (c Client) Remove(ctx context.Context, params *RemoveParams) error { return clients.ErrMustSpecifyOrg } - orgId := params.OrgId - if orgId == "" { - orgName := params.OrgName - if orgName == "" { - orgName = c.ActiveConfig.Org - } - res, err := c.GetOrgs(ctx).Org(orgName).Execute() - if err != nil { - return fmt.Errorf("failed to lookup org with name %q: %w", orgName, err) - } - if len(res.GetOrgs()) == 0 { - return fmt.Errorf("no organization with name %q: %w", orgName, err) - } - orgId = res.GetOrgs()[0].GetId() + orgId, err := c.GetOrgId(ctx, params.OrgId, params.OrgName, c.OrganizationsApi) + if err != nil { + return err } stacks, err := c.ListStacks(ctx).OrgID(orgId).StackID(params.Ids).Execute() diff --git a/clients/telegrafs/telegrafs.go b/clients/telegrafs/telegrafs.go index da517ec..f2f7e31 100644 --- a/clients/telegrafs/telegrafs.go +++ b/clients/telegrafs/telegrafs.go @@ -156,20 +156,5 @@ func (c Client) printTelegrafs(opts telegrafPrintOpts) error { } func (c Client) getOrgID(ctx context.Context, orgID influxid.ID, orgName string) (string, error) { - if orgID.Valid() { - return orgID.String(), nil - } else { - if orgName == "" { - orgName = c.ActiveConfig.Org - } - resp, err := c.GetOrgs(ctx).Org(orgName).Execute() - if err != nil { - return "", fmt.Errorf("failed to lookup ID of org %q: %w", orgName, err) - } - orgs := resp.GetOrgs() - if len(orgs) == 0 { - return "", fmt.Errorf("no organization found with name %q", orgName) - } - return orgs[0].GetId(), nil - } + return c.GetOrgIdI(ctx, orgID, orgName, c.OrganizationsApi) } diff --git a/clients/template/template.go b/clients/template/template.go index 6d8dc46..64b087b 100644 --- a/clients/template/template.go +++ b/clients/template/template.go @@ -26,23 +26,9 @@ type SummarizeParams struct { } func (c Client) Summarize(ctx context.Context, params *SummarizeParams) error { - if params.OrgId == "" && params.OrgName == "" && c.ActiveConfig.Org == "" { - return clients.ErrMustSpecifyOrg - } - orgID := params.OrgId - if orgID == "" { - orgName := params.OrgName - if orgName == "" { - orgName = c.ActiveConfig.Org - } - orgs, err := c.GetOrgs(ctx).Org(orgName).Execute() - if err != nil { - return fmt.Errorf("failed to get ID for org %q: %w", orgName, err) - } - if len(orgs.GetOrgs()) == 0 { - return fmt.Errorf("no orgs found with name %q: %w", orgName, err) - } - orgID = orgs.GetOrgs()[0].GetId() + orgID, err := c.GetOrgId(ctx, params.OrgId, params.OrgName, c.OrganizationsApi) + if err != nil { + return err } templates, err := template.ReadSources(ctx, params.Sources) @@ -75,23 +61,9 @@ type ValidateParams struct { } func (c Client) Validate(ctx context.Context, params *ValidateParams) error { - if params.OrgId == "" && params.OrgName == "" && c.ActiveConfig.Org == "" { - return clients.ErrMustSpecifyOrg - } - orgID := params.OrgId - if orgID == "" { - orgName := params.OrgName - if orgName == "" { - orgName = c.ActiveConfig.Org - } - orgs, err := c.GetOrgs(ctx).Org(orgName).Execute() - if err != nil { - return fmt.Errorf("failed to get ID for org %q: %w", orgName, err) - } - if len(orgs.GetOrgs()) == 0 { - return fmt.Errorf("no orgs found with name %q: %w", orgName, err) - } - orgID = orgs.GetOrgs()[0].GetId() + orgID, err := c.GetOrgId(ctx, params.OrgId, params.OrgName, c.OrganizationsApi) + if err != nil { + return err } templates, err := template.ReadSources(ctx, params.Sources) diff --git a/clients/user/user.go b/clients/user/user.go index 7bb6873..8b6c9c8 100644 --- a/clients/user/user.go +++ b/clients/user/user.go @@ -25,36 +25,12 @@ type CreateParams struct { var ErrMustSpecifyUser = errors.New("must specify user ID or user name") -func getOrgID(ctx context.Context, params *clients.OrgParams, c clients.CLI, orgApi api.OrganizationsApi) (string, error) { - if !params.OrgID.Valid() && params.OrgName == "" && c.ActiveConfig.Org == "" { - return "", clients.ErrMustSpecifyOrg - } - orgID := params.OrgID.String() - if !params.OrgID.Valid() { - req := orgApi.GetOrgs(ctx) - if params.OrgName != "" { - req = req.Org(params.OrgName) - } else { - req = req.Org(c.ActiveConfig.Org) - } - orgs, err := req.Execute() - if err != nil { - return "", fmt.Errorf("failed to find org %q: %w", params.OrgName, err) - } - if orgs.Orgs == nil || len(*orgs.Orgs) == 0 { - return "", fmt.Errorf("no org found with name %q", params.OrgName) - } - orgID = (*orgs.Orgs)[0].GetId() - } - return orgID, nil -} - func (c Client) Create(ctx context.Context, params *CreateParams) error { if params.Password != "" && len(params.Password) < stdio.MinPasswordLen { return clients.ErrPasswordIsTooShort } - orgID, err := getOrgID(ctx, ¶ms.OrgParams, c.CLI, c.OrganizationsApi) + orgID, err := c.GetOrgIdI(ctx, params.OrgID, params.OrgName, c.OrganizationsApi) if err != nil { return err } diff --git a/clients/user/user_test.go b/clients/user/user_test.go index f36a22e..7ac87fd 100644 --- a/clients/user/user_test.go +++ b/clients/user/user_test.go @@ -211,7 +211,7 @@ func TestClient_Create(t *testing.T) { return assert.Equal(t, "my-default-org", *in.GetOrg()) })).Return(api.Organizations{Orgs: &[]api.Organization{}}, nil) }, - expectedErr: "no org found", + expectedErr: `no organization with name "my-default-org"`, }, { name: "assigning membership failed", diff --git a/clients/v1_auth/v1_auth.go b/clients/v1_auth/v1_auth.go index 8ed8cf8..4efbb4c 100644 --- a/clients/v1_auth/v1_auth.go +++ b/clients/v1_auth/v1_auth.go @@ -357,9 +357,9 @@ func (c Client) printV1Tokens(params *v1PrintOpts) error { headers := []string{ "ID", "Description", - "Name / Token", - "User Name", - "User ID", + "Username", + "v2 User Name", + "v2 User ID", "Permissions", } if params.deleted { @@ -374,9 +374,9 @@ func (c Client) printV1Tokens(params *v1PrintOpts) error { row := map[string]interface{}{ "ID": u.ID, "Description": u.Description, - "Name / Token": u.Token, - "User Name": u.UserName, - "User ID": u.UserID, + "Username": u.Token, + "v2 User Name": u.UserName, + "v2 User ID": u.UserID, "Permissions": u.Permissions, } if params.deleted { @@ -403,22 +403,5 @@ func (c Client) getAuthReqID(ctx context.Context, params *AuthLookupParams) (id } func (c Client) getOrgID(ctx context.Context, params clients.OrgParams) (string, error) { - if !params.OrgID.Valid() && params.OrgName == "" && c.ActiveConfig.Org == "" { - return "", clients.ErrMustSpecifyOrg - } - if params.OrgID.Valid() { - return params.OrgID.String(), nil - } - name := params.OrgName - if name == "" { - name = c.ActiveConfig.Org - } - org, err := c.GetOrgs(ctx).Org(name).Execute() - if err != nil { - return "", fmt.Errorf("failed to lookup org with name %q: %w", name, err) - } - if len(org.GetOrgs()) == 0 { - return "", fmt.Errorf("no organization with name %q: %w", name, err) - } - return org.GetOrgs()[0].GetId(), nil + return c.GetOrgIdI(ctx, params.OrgID, params.OrgName, c.OrganizationsApi) } diff --git a/cmd/influx/global.go b/cmd/influx/global.go index 288aa56..66d0f6b 100644 --- a/cmd/influx/global.go +++ b/cmd/influx/global.go @@ -3,6 +3,7 @@ package main import ( "context" "fmt" + "io" "net/url" "runtime" @@ -161,6 +162,14 @@ func getAPINoToken(ctx *cli.Context) *api.APIClient { return i } +type CommonBoolFlag struct { + cli.BoolFlag +} + +type CommonStringFlag struct { + cli.StringFlag +} + // NOTE: urfave/cli has dedicated support for global flags, but it only parses those flags // if they're specified before any command names. This is incompatible with the old influx // CLI, which repeatedly registered common flags on every "leaf" command, forcing the flags @@ -170,65 +179,107 @@ func getAPINoToken(ctx *cli.Context) *api.APIClient { // configPathFlag returns the flag used by commands that access the CLI's local config store. func configPathFlag() cli.Flag { - return &cli.StringFlag{ - Name: configPathFlagName, - Usage: "Path to the influx CLI configurations", - EnvVar: "INFLUX_CONFIGS_PATH", + return &CommonStringFlag{ + cli.StringFlag{ + Name: configPathFlagName, + Usage: "Path to the influx CLI configurations", + EnvVar: "INFLUX_CONFIGS_PATH", + }, } } // coreFlags returns flags used by all CLI commands that make HTTP requests. func coreFlags() []cli.Flag { return []cli.Flag{ - &cli.StringFlag{ + &CommonStringFlag{cli.StringFlag{ Name: hostFlagName, Usage: "HTTP address of InfluxDB", EnvVar: "INFLUX_HOST", - }, - &cli.BoolFlag{ + }}, + &CommonBoolFlag{cli.BoolFlag{ Name: skipVerifyFlagName, Usage: "Skip TLS certificate chain and host name verification", EnvVar: "INFLUX_SKIP_VERIFY", - }, + }}, configPathFlag(), - &cli.StringFlag{ + &CommonStringFlag{cli.StringFlag{ Name: configNameFlagName + ", c", Usage: "Config name to use for command", EnvVar: "INFLUX_ACTIVE_CONFIG", - }, - &cli.StringFlag{ + }}, + &CommonStringFlag{cli.StringFlag{ Name: traceIdFlagName, Hidden: true, EnvVar: "INFLUX_TRACE_DEBUG_ID", - }, - &cli.BoolFlag{ + }}, + &CommonBoolFlag{cli.BoolFlag{ Name: httpDebugFlagName, - }, + }}, } } // printFlags returns flags used by commands that display API resources to the user. func printFlags() []cli.Flag { return []cli.Flag{ - &cli.BoolFlag{ + &CommonBoolFlag{cli.BoolFlag{ Name: printJsonFlagName, Usage: "Output data as JSON", EnvVar: "INFLUX_OUTPUT_JSON", - }, - &cli.BoolFlag{ + }}, + &CommonBoolFlag{cli.BoolFlag{ Name: hideHeadersFlagName, Usage: "Hide the table headers in output data", EnvVar: "INFLUX_HIDE_HEADERS", - }, + }}, } } // commonTokenFlag returns the flag used by commands that hit an authenticated API. func commonTokenFlag() cli.Flag { - return &cli.StringFlag{ + return &CommonStringFlag{cli.StringFlag{ Name: tokenFlagName + ", t", - Usage: "Authentication token", + Usage: "Token to authenticate request", EnvVar: "INFLUX_TOKEN", + }} +} + +func init() { + + // SubcommandHelpTemplate is the text template for the subcommand help topic. + // cli.go uses text/template to render templates. You can + // render custom help text by setting this variable. + cli.CommandHelpTemplate = `NAME: + {{.HelpName}} - {{.Usage}} + +USAGE: + {{if .UsageText}}{{.UsageText}}{{else}}{{.HelpName}}{{if .VisibleFlags}} [command options]{{end}} {{if .ArgsUsage}}{{.ArgsUsage}}{{else}}[arguments...]{{end}}{{end}}{{if .Category}} + +CATEGORY: + {{.Category}}{{end}}{{if .Description}} + +DESCRIPTION: + {{.Description}}{{end}}{{if .VisibleFlags}} + +COMMON OPTIONS: + {{range .VisibleFlags}}{{if iscommon .}}{{.}} + {{end}}{{end}} +OPTIONS: + {{range .VisibleFlags}}{{if not (iscommon .)}}{{.}} + {{end}}{{end}}{{end}} +` + cli.HelpPrinter = func(w io.Writer, templ string, data interface{}) { + customFunc := make(map[string]interface{}) + customFunc["iscommon"] = func(flag cli.Flag) bool { + switch flag.(type) { + case CommonBoolFlag: + return true + case CommonStringFlag: + return true + default: + return false + } + } + cli.HelpPrinterCustom(w, templ, data, customFunc) } }