Update px cli to make `--cloud_addr` a required argument #1960

ddelnano · 2024-07-02T18:37:13Z

Summary: Update px cli to make --cloud_addr a required argument

This removes the CC4P dependency on the Pixie cli so that the project is agnostic to any vendor provided Pixie cloud. While this argument is now required, if the cli is run within an interactive shell, the user will be presented with the available Pixie clouds and have the opportunity to select the one they want to use (see Test Plan below).

My plan is to take this a step further for the interactive case and have the cli populate the ~/.pixie/config.json (or similar) file if the user wants to save their selected cloud. Future invocations of the cli will use what is stored in ~/.pixie.

Relevant Issues: N/A

Type of change: /kind cleanup

Test Plan: Tested the following scenarios

px help, px --help, px deploy --help (command) and px script help (command with sub commands) bypass the new required logic. Note: px deploy help would require the argument since it is an invalid command (deploy doesn't have a help subcommand)
Running non interactively exits if --cloud_addr has not been provided

$ ssh host-with-pixie-cli "/bin/bash -c '~/code/pixie/px deploy help'"
Pixie CLI
No cloud address provided. Please set the cloud address using the `--cloud_addr` flag or `PX_CLOUD_ADDR` environment variable.

Running interactively shows the list of possible clouds

$ bazel run src/pixie_cli:px -- deploy
INFO: Analyzed target //src/pixie_cli:px (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //src/pixie_cli:px up-to-date:
  bazel-bin/src/pixie_cli/px_/px
INFO: Elapsed time: 0.309s, Critical Path: 0.02s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
INFO: Running command line: bazel-bin/src/pixie_cli/px_/px deploy
Pixie CLI
Use the arrow keys to navigate: ↓ ↑ → ←
? Select Pixie cloud:
  ▸ withpixie.ai:443

Running command that requires cloud addr succeeds

Changelog Message: Update px cli to make --cloud_addr a required argument if not populated by the PX_CLOUD_ADDR environment variable

Signed-off-by: Dom Del Nano <[email protected]>

src/pixie_cli/pkg/cmd/run.go

Signed-off-by: Dom Del Nano <[email protected]>

ddelnano · 2024-07-09T21:46:27Z

@vihangm this is ready for another round of review when you have the chance.

src/pixie_cli/pkg/cmd/root.go

Signed-off-by: Dom Del Nano <[email protected]>

…n pixie config file (#1964) Summary: Allow interactive px cli usage to prompt and save preferred cloud in pixie config file This is a continuation of the plan I outlined in #1960 now that cloud addr is required for the `px` cli. Relevant Issues: N/A Type of change: /kind feature Test Plan: Verified the following scenarios. I will make sure to update the PX cli release checklist accordingly if/when approved <details><summary>Default cloud selection testing</summary> - [x] Running `px` command that requires cloud prompts for selection and whether to store it ``` $ ./px auth login Pixie CLI ✔ withpixie.ai:443 ✔ No <------ Cloud selection was opted out of Starting browser... (if browser-based login fails, try running `px auth login --manual` for headless login) Fetching refresh token ... Failed to perform browser based auth. Will try manual auth error=browser failed to open Please Visit: https://work.withpixie.ai:443/login?local_mode=true Copy and paste token here: ^C ``` - [x] Cli commands following storing preferred cloud use the preferred value ``` $ ./px auth login Pixie CLI ✔ withpixie.ai:443 ✔ Yes <------ Cloud selection was opted into saving Starting browser... (if browser-based login fails, try running `px auth login --manual` for headless login) Fetching refresh token ... Failed to perform browser based auth. Will try manual auth error=browser failed to open Please Visit: https://work.withpixie.ai:443/login?local_mode=true Copy and paste token here: ^C $ ./px auth login Pixie CLI Starting browser... (if browser-based login fails, try running `px auth login --manual` for headless login) Fetching refresh token ... Failed to perform browser based auth. Will try manual auth error=browser failed to open Please Visit: https://work.withpixie.ai:443/login?local_mode=true Copy and paste token here: ^C $ cat ~/.pixie/config.json {"uniqueClientID":"XXX","cloudAddr":"withpixie.ai:443"} ``` - [x] Using `--cloud_addr` overrides the value set in config file ``` $ cat ~/.pixie/config.json {"uniqueClientID":"XXX","cloudAddr":"boguscloud.com"} $ ./px --cloud_addr=withpixie.ai auth login Pixie CLI Starting browser... (if browser-based login fails, try running `px auth login --manual` for headless login) Fetching refresh token ... Failed to perform browser based auth. Will try manual auth error=browser failed to open Please Visit: https://work.withpixie.ai:443/login?local_mode=true Copy and paste token here: ``` - [x] Running non-interactively uses cloud stored in config file - [x] Running non-interactively without preferred cloud or `--cloud_addr` results in error </details> <details><summary>`px config` command testing</summary> - [x] `px config list` prints out cloud addr ``` $ ./px config list Pixie CLI CloudAddr: boguscloud.com ``` - [x] `px config set` validates arguments ``` $ ./px config set --key NonExistant --value tesitng Pixie CLI FATA[0000]src/pixie_cli/pkg/cmd/config.go:80 px.dev/pixie/src/pixie_cli/pkg/cmd.glob..func16() Key 'NonExistant' is not settable. Must be one of [CloudAddr] $ ./px config set --key CloudAddr --value withpixie.ai:443 --value testing Pixie CLI FATA[0000]src/pixie_cli/pkg/cmd/config.go:74 px.dev/pixie/src/pixie_cli/pkg/cmd.glob..func16() the number of --key and --value flags must match ``` - [x] `px config set` updates config file ``` $ ./px config set --key CloudAddr --value withpixie.ai:443 Pixie CLI $ ./px config list Pixie CLI CloudAddr: withpixie.ai:443 ``` </details> Changelog Message: Update `px` cli to store preferred cloud in pixie config file Signed-off-by: Dom Del Nano <[email protected]>

GitOrigin-RevId: bbb90a3

…n pixie config file (pixie-io#1964) Summary: Allow interactive px cli usage to prompt and save preferred cloud in pixie config file This is a continuation of the plan I outlined in pixie-io#1960 now that cloud addr is required for the `px` cli. Relevant Issues: N/A Type of change: /kind feature Test Plan: Verified the following scenarios. I will make sure to update the PX cli release checklist accordingly if/when approved <details><summary>Default cloud selection testing</summary> - [x] Running `px` command that requires cloud prompts for selection and whether to store it ``` $ ./px auth login Pixie CLI ✔ withpixie.ai:443 ✔ No <------ Cloud selection was opted out of Starting browser... (if browser-based login fails, try running `px auth login --manual` for headless login) Fetching refresh token ... Failed to perform browser based auth. Will try manual auth error=browser failed to open Please Visit: https://work.withpixie.ai:443/login?local_mode=true Copy and paste token here: ^C ``` - [x] Cli commands following storing preferred cloud use the preferred value ``` $ ./px auth login Pixie CLI ✔ withpixie.ai:443 ✔ Yes <------ Cloud selection was opted into saving Starting browser... (if browser-based login fails, try running `px auth login --manual` for headless login) Fetching refresh token ... Failed to perform browser based auth. Will try manual auth error=browser failed to open Please Visit: https://work.withpixie.ai:443/login?local_mode=true Copy and paste token here: ^C $ ./px auth login Pixie CLI Starting browser... (if browser-based login fails, try running `px auth login --manual` for headless login) Fetching refresh token ... Failed to perform browser based auth. Will try manual auth error=browser failed to open Please Visit: https://work.withpixie.ai:443/login?local_mode=true Copy and paste token here: ^C $ cat ~/.pixie/config.json {"uniqueClientID":"XXX","cloudAddr":"withpixie.ai:443"} ``` - [x] Using `--cloud_addr` overrides the value set in config file ``` $ cat ~/.pixie/config.json {"uniqueClientID":"XXX","cloudAddr":"boguscloud.com"} $ ./px --cloud_addr=withpixie.ai auth login Pixie CLI Starting browser... (if browser-based login fails, try running `px auth login --manual` for headless login) Fetching refresh token ... Failed to perform browser based auth. Will try manual auth error=browser failed to open Please Visit: https://work.withpixie.ai:443/login?local_mode=true Copy and paste token here: ``` - [x] Running non-interactively uses cloud stored in config file - [x] Running non-interactively without preferred cloud or `--cloud_addr` results in error </details> <details><summary>`px config` command testing</summary> - [x] `px config list` prints out cloud addr ``` $ ./px config list Pixie CLI CloudAddr: boguscloud.com ``` - [x] `px config set` validates arguments ``` $ ./px config set --key NonExistant --value tesitng Pixie CLI FATA[0000]src/pixie_cli/pkg/cmd/config.go:80 px.dev/pixie/src/pixie_cli/pkg/cmd.glob..func16() Key 'NonExistant' is not settable. Must be one of [CloudAddr] $ ./px config set --key CloudAddr --value withpixie.ai:443 --value testing Pixie CLI FATA[0000]src/pixie_cli/pkg/cmd/config.go:74 px.dev/pixie/src/pixie_cli/pkg/cmd.glob..func16() the number of --key and --value flags must match ``` - [x] `px config set` updates config file ``` $ ./px config set --key CloudAddr --value withpixie.ai:443 Pixie CLI $ ./px config list Pixie CLI CloudAddr: withpixie.ai:443 ``` </details> Changelog Message: Update `px` cli to store preferred cloud in pixie config file Signed-off-by: Dom Del Nano <[email protected]> GitOrigin-RevId: 1d41f9b

Update px cli to make --cloud_addr a required argument

8d37f68

Signed-off-by: Dom Del Nano <[email protected]>

ddelnano requested review from a team as code owners July 2, 2024 18:37

Fix linting issues

95e6408

Signed-off-by: Dom Del Nano <[email protected]>

vihangm reviewed Jul 3, 2024

View reviewed changes

src/pixie_cli/pkg/cmd/run.go Outdated Show resolved Hide resolved

Remove use of potentially unbound flag and simplify help text

33eab07

Signed-off-by: Dom Del Nano <[email protected]>

vihangm reviewed Jul 9, 2024

View reviewed changes

src/pixie_cli/pkg/cmd/root.go Outdated Show resolved Hide resolved

Update comment to reflect actual logic

b396071

Signed-off-by: Dom Del Nano <[email protected]>

vihangm approved these changes Jul 10, 2024

View reviewed changes

ddelnano merged commit bbb90a3 into pixie-io:main Jul 10, 2024
31 checks passed

ddelnano mentioned this pull request Jul 10, 2024

Allow interactive px cli usage to prompt and save preferred cloud in pixie config file #1964

Merged

8 tasks

ddelnano added a commit to ddelnano/pixie that referenced this pull request Sep 23, 2024

Update px cli to make --cloud_addr a required argument (pixie-io#1960)

f46bbdb

GitOrigin-RevId: bbb90a3

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update px cli to make `--cloud_addr` a required argument #1960

Update px cli to make `--cloud_addr` a required argument #1960

ddelnano commented Jul 2, 2024 •

edited

Loading

ddelnano commented Jul 9, 2024

Update px cli to make --cloud_addr a required argument #1960

Update px cli to make --cloud_addr a required argument #1960

Conversation

ddelnano commented Jul 2, 2024 • edited Loading

ddelnano commented Jul 9, 2024

Update px cli to make `--cloud_addr` a required argument #1960

Update px cli to make `--cloud_addr` a required argument #1960

ddelnano commented Jul 2, 2024 •

edited

Loading