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

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

Merged

Conversation

ddelnano
Copy link
Member

@ddelnano ddelnano commented Jul 10, 2024

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

Default cloud selection testing
  • 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
  • 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"}
  • 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:
  • Running non-interactively uses cloud stored in config file
  • Running non-interactively without preferred cloud or --cloud_addr results in error
`px config` command testing
  • px config list prints out cloud addr
$ ./px config list
Pixie CLI
CloudAddr: boguscloud.com
  • 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
  • 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

Changelog Message: Update px cli to store preferred cloud in pixie config file

@ddelnano ddelnano requested review from a team as code owners July 10, 2024 17:00
@ddelnano ddelnano changed the title Allow interactive px cli usage to prompt for saving preferred cloud in pixie config file Allow interactive px cli usage to prompt and save preferred cloud in pixie config file Jul 10, 2024
@ddelnano ddelnano force-pushed the ddelnano/add-cloud_addr-to-pixie-config-file branch from 915e759 to 37612a4 Compare July 10, 2024 17:01
@ddelnano ddelnano merged commit 1d41f9b into pixie-io:main Jul 16, 2024
35 checks passed
@ddelnano ddelnano deleted the ddelnano/add-cloud_addr-to-pixie-config-file branch July 16, 2024 21:41
ddelnano added a commit to ddelnano/pixie that referenced this pull request Aug 23, 2024
ddelnano added a commit to ddelnano/pixie that referenced this pull request Aug 23, 2024
… cloud in pixie config file (pixie-io#1964)"

This reverts commit 1d41f9b.

Signed-off-by: Dom Del Nano <[email protected]>
aimichelle pushed a commit that referenced this pull request Aug 23, 2024
…ile (#1964) (#1990)

Summary: Revert change for interactive prompt to store cloud to pixie
config file (#1964)

We've received feedback from users that is confusing to no longer have a
default cloud for the pixie cli. We will be restoring the previous
behavior in a follow up PR. With that new direction in mind, it no
longer makes sense to have the functionality added in #1964.

Relevant Issues: N/A

Type of change: /kind cleanup

Test Plan: None since this was a `git revert`

Signed-off-by: Dom Del Nano <[email protected]>
ddelnano added a commit to ddelnano/pixie that referenced this pull request Sep 23, 2024
…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
ddelnano added a commit to ddelnano/pixie that referenced this pull request Sep 23, 2024
…ile (pixie-io#1964) (pixie-io#1990)

Summary: Revert change for interactive prompt to store cloud to pixie
config file (pixie-io#1964)

We've received feedback from users that is confusing to no longer have a
default cloud for the pixie cli. We will be restoring the previous
behavior in a follow up PR. With that new direction in mind, it no
longer makes sense to have the functionality added in pixie-io#1964.

Relevant Issues: N/A

Type of change: /kind cleanup

Test Plan: None since this was a `git revert`

Signed-off-by: Dom Del Nano <[email protected]>
GitOrigin-RevId: 33244cd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants