influxdata / influx-cli

CLI for managing resources in InfluxDB v2
MIT License
65 stars 24 forks source link

config vs. env var precedence in influx CLI can be confusing #92

Open timhallinflux opened 3 years ago

timhallinflux commented 3 years ago

Steps to reproduce: Using nightly build of Windows CLI.

  1. create config profile using influx to connect to local influxdb instance using all access token
  2. attempt to execute any command via influx such as influx bucket list or influx v1 auth create -- results in unauthorized
  3. however, using all the same params (org, token, url) without the config profile, these commands work just fine.

Example:

C:\Program Files\InfluxData\influxdb2-new>influx bucket list --host http://localhost:8086 -o thall-org -t 9HrCnWDsD0Adhq...rest_of_the_all_access_token=
ID                      Name            Retention       Shard group duration    Organization ID
782886daf164bae1        _monitoring     168h0m0s        24h0m0s                 5ecbfa70b25f4cc9
5ecbfa70b25f4cc9        _tasks          72h0m0s         24h0m0s                 5ecbfa70b25f4cc9
760dc052f12e7c80        oss_metrics     168h0m0s        24h0m0s                 5ecbfa70b25f4cc9
a084ffba216305be        telegraf        2160h0m0s       24h0m0s                 5ecbfa70b25f4cc9

C:\Program Files\InfluxData\influxdb2-new>influx config list
Active  Name    URL                     Org
*       local   http://localhost:8086   thall-org

C:\Program Files\InfluxData\influxdb2-new>influx config set -n local -t 9HrCnWDsD0Adhq...rest_of_the_all_access_token=
Active  Name    URL                     Org
*       local   http://localhost:8086   thall-org

C:\Program Files\InfluxData\influxdb2-new>influx bucket list
Error: Failed to retrieve buckets: unauthorized access.
See 'influx bucket list -h' for help

Expected behavior: Config profiles should work if configured correctly.

Actual behavior: Unauthorized, despite using params that appear to work otherwise

Environment info:

williamhbaker commented 3 years ago

Hi Tim,

I'm not able to reproduce this locally. Using the April 15th nightly windows build running on windows 10 with the windows CLI - this is starting "fresh" and using the quick start through the onboarding, then creating a new all-access token through the UI and using that in the commands - everything seems to work as expected:

C:\Program Files\influxdb2-new>influx bucket list -o wbaker-org -t cHA1SC0DQ3[...etc]
ID                      Name            Retention       Shard group duration    Organization ID
1c8d9a8272ecbd70        _monitoring     168h0m0s        24h0m0s                 82774d332f700abd
82774d332f700abd        _tasks          72h0m0s         24h0m0s                 82774d332f700abd
542923c69e43829e        wbaker-bucket   infinite        168h0m0s                82774d332f700abd

C:\Program Files\influxdb2-new>influx config create --active -n local -o wbaker-org -u http://localhost:8086 -t cHA1SC0DQ3[...etc]
Active  Name    URL                     Org
*       local   http://localhost:8086   wbaker-org

C:\Program Files\influxdb2-new>influx config list
Active  Name    URL                     Org
*       local   http://localhost:8086   wbaker-org

C:\Program Files\influxdb2-new>influx config set -n local -t cHA1SC0DQ3[...etc]
Active  Name    URL                     Org
*       local   http://localhost:8086   wbaker-org

C:\Program Files\influxdb2-new>influx bucket list
ID                      Name            Retention       Shard group duration    Organization ID
1c8d9a8272ecbd70        _monitoring     168h0m0s        24h0m0s                 82774d332f700abd
82774d332f700abd        _tasks          72h0m0s         24h0m0s                 82774d332f700abd
542923c69e43829e        wbaker-bucket   infinite        168h0m0s                82774d332f700abd

The latest nightly build includes some additional logging that might be helpful to get a better idea of what's going on - would you be able to try with that and see what it says? I've tried with that one as well and can't reproduce your error either.

timhallinflux commented 3 years ago

We've tracked this down. Currently environment variables are taking precedent over the config profile. There was a token set in an environment variable --- and the URL and org were not. Hence, the CLI picks the environment variables first...and then applied the config profile.

I would propose a few changes as a result of this: 1) config profile should be used first -- and it should use ALL of the elements of that profile (org, token, URL). 2) in the event that the config profile settings are incomplete or incorrect, that should present the user with an error -- which guides them to review the config profile attributes. 3) env vars should be used if: a) there is not an active config profile or b) there are missing parameters (i.e. not all the required parameters are supplied on the command line) 4) documentation should describe the behavior ---

dgnorton commented 3 years ago

The current behavior (env var overrides config file) is consistent with InfluxDB.

The environment variable overrides the equivalent option in the configuration file.

timhallinflux commented 3 years ago

interesting, however...since the config profile is something that is created through the CLI, it is a bit unexpected from an end user perspective.

dgnorton commented 3 years ago

I don't think there's a standard but the current order of precedence seems consistent with a quick survey of other apps. E.g., AWS CLI, Docker, MongoDB, git, npm, Ansible. As I recall from my years in the Windows world, cmd line args -> env vars -> Registry -> config file is the usual order.

timhallinflux commented 3 years ago

yeah...except there is a pretty big difference here. There is an entire section of the CLI dedicated to building 1 or more config profiles -- which is suppose to allow for the developer to easily switch between them while working with the CLI. If there is an env var which overrides all of the config profiles, the feature is pretty useless. The config profile is more of a short cut to avoid excessive typing.

sanderson commented 3 years ago

Another use case to Tim's point – using stdout from one InfluxDB instance to write to a separate InfluxDB instance:

influxd inspect export-lp \
  --active-config local \
  --bucket example-bucket \
  --output - \
| influx write \
  --active-config cloud \
  --bucket example-bucket

As is, if a user has any of the host, org, or token evars set, this command will fail to authenticate with at least one of the hosts because the credentials are overridden by the evar.

sanderson commented 3 years ago

I think one way to look at these CLI configs is that they're a shortcut for CLI flags. With influxd, command flags take precedence over environment variables. I believe it should be the same in this case.

danxmoran commented 3 years ago

Piping local -> cloud is compelling to me, but as a counter-point to some of the details:

influx setup will init the configs file with a fully-formed profile, and mark it as active. I never create additional configs, because the only thing I change while experimenting/reproducing issues is the token value. If I'm doing a one-off command I'll sometimes pass -t, but if I'm running a series of commands I'll create a fresh shell and export INFLUX_TOKEN to save typing. AFAICS @timhallinflux's proposed precedence rules would break this workflow, because the config on disk would always be fully-formed. I'd be annoyed if I had to create & switch to a new config profile each time, because I'd have to think of a new profile name and remember to clean up my local configs file once I was done with a token.

In the local -> cloud case, I see --active-config is explicitly set in each case. Could we adjust the precedence rules to be:

  1. If --active-config is set, ignore all env vars and use whatever's in the profile
  2. Otherwise, use the existing behavior

In my head that'd help reduce confusion/frustration for multi-profile users, while minimizing the breakage for single-profile users like me.

timhallinflux commented 3 years ago

so --active-config would be parameter-less --- if you want to force the use of the current active config explicitly. if you want to override the active config with any other config for a 1 off call... --active-config <profilename> can be supplied. if you leave --active-config off, the current behavior which is look for env vars first, then default to active config?

Some output here would be useful -- in terms of debug logging about what was used when a command was issued.

danxmoran commented 3 years ago

:+1: that sounds reasonable to me. Agreed that some debug info would be helpful.

Do we want to hold 2.0.5 for this? It's not really specific to the new Windows build (renamed the issue accordingly). I'm working on splitting the CLI into a separate repo, the behavior could be changed as part of that work.

timhallinflux commented 3 years ago

No. I wouldn't hold the release for this.

samhld commented 3 years ago

@danxmoran instead of me creating a new issue, I'll just add this here:

When an influx CLI command requires an organization identifier, it takes only one of flags --org or --org-id and only one of env vars INFLUX_ORG OR INFLUX_ORG_ID. If both env vars are set, the user gets an error saying they need to choose only one...even if they use one of the above flags to "override" this.

Example:

influx v1 dbrp create --org-id b92bac306fa58e57 --bucket-id dd1fd2e3868f604d --db mqtt --rp default

Yields...

Error: Must specify org-id, or org name not both.

....when both org ID and org name are set in the environment.

Both can be set because they're each needed for different things. For instance, INFLUX_ORG is useful as a persistent and human-readable/human-memorable name. Org ID is useful as a more unique identifier when necessary.

jacobmarble commented 2 years ago

I ran into this today. A few env vars from months ago happened to live in my .zshenv. No amount of influx config create or editing ~/.influxdbv2/configs seemed to help. Didn't see a --verbose flag documented.

Figured it out eventually, though.

timhallinflux commented 2 years ago

A --verbose flag would be handy to dump out both the source and the values associated with those pesky variables. Good suggestion.

samhld commented 2 years ago

@timhallinflux @jacobmarble we have --http-debug which will show you details but it doesn't include org info. @DStrand1 @jeffreyssmith2nd what would it take to add org/org_id info in http-debug output?

jeffreyssmith2nd commented 2 years ago

It currently shows it for api calls that filter on the org, it's just a little buried. You can see it in the query string here:

2022/07/15 09:09:53                        
GET /api/v2/buckets?limit=20&org=b4f0e8d1214e92a0 HTTP/1.1
Host: us-east-1-1.aws.cloud2.influxdata.com                                                                     
User-Agent: influx/dev (darwin) Sha/none Date/2022-07-12T20:34:25Z
Accept: application/json         
Authorization: Token 7On7za8D3euuf7T-hZaqjzyTWVqX6Rz-t4FJiMdKz9HwSFGmcQki5H62lmD6Y3UnLdJg4X8sVxQ1yPTY1kJpdw==
Accept-Encoding: gzip