upbound / up

The @upbound CLI
Apache License 2.0
52 stars 41 forks source link

Overhaul all commands to use stripped profiles and kubecontexts #471

Closed RedbackThomson closed 5 months ago

RedbackThomson commented 5 months ago

Description of your changes

This pull request is the first step toward a larger paradigm shift in the way we use profiles and kube contexts to achieve a new, more unified user experience.

Customers will use the up CLI in two different ways:

Migrating to this new model allows for code paths to be greatly simplified, and the user experience to remain identical between self-hosted and cloud. Only up ctx will differ between the two environments, as cloud has higher levels of hierarchy (namely organizations and spaces). All commands run after setting context will be agnostic to the underlying host of the space (be it connected, managed or self-hosted).

This new mechanism eliminates the need for up ctp kubeconfig get and up ctp connect, replaced with the equivalent up ctx command. It also eliminates the need for most fields within the profiles, leaving only the token, account field and token type, which are unfortunately still needed to support commands like up robot create and up configuration create.

Note: This PR does not produce a remotely functional or tested CLI, yet. Also I have not updated any of the tests, so all tests and the linting fails.

I have:

How has this code been tested

tnthornton commented 5 months ago

This new mechanism eliminates the need for up ctp kubeconfig get and up ctp connect

Still need to look at the PR, but how does one get a kubeconfig for say 'Argo' to point at a control plane with this paradigm?

RedbackThomson commented 5 months ago

This new mechanism eliminates the need for up ctp kubeconfig get and up ctp connect

Still need to look at the PR, but how does one get a kubeconfig for say 'Argo' to point at a control plane with this paradigm?

up ctx does the same thing as up ctp kubeconfig (provided you have already set the session token in a profile):

up ctx --kubeconfig out.kubeconfig upbound-aws-us-east-1/my-group/my-ctp
ezgidemirel commented 5 months ago

Thanks a lot @RedbackThomson for putting together this PR. I've updated some parts in here PR and tested some of the commands against a self-hosted space.

https://github.com/upbound/up/assets/29144823/e1023a26-08ec-40b7-b18f-1e6303793943

https://github.com/upbound/up/assets/29144823/c7e0da21-cd2d-4505-a02d-d27096eeb8f1

https://github.com/upbound/up/assets/29144823/f35e64fb-3166-47f3-96bc-330fcbe71e4e

For up ctx command, I've noticed some differences with the previous implementation and discrepancies in context names while switching. I'm not sure if these are intended or not:

https://github.com/upbound/up/assets/29144823/4e405c8f-f19c-489f-a93e-694368ae1f88

erhancagirici commented 5 months ago

Some findings from my side regarding up ctx at the current state of the code:

  1. Not a bug but questionable UX: up ctx invocations always starts the navigation from org selection level, even when you are on some valid group level. I'd expect subsequent up ctx invocations would start navigation from the current level I was currently in. Not sure if this intended, at least this was like that before.

  2. When you select a controlplane via up ctx navigation, subsequent up ctx does not allow navigating anymore:

    • Error: no matches for kind "ControlPlane" in version "spaces.upbound.io/v1beta1" message is shown in interactive navigation
    • up ctx .. returns with no error and does not change the context.
    • up ctx - did not restore the previous context either, so basically I was stuck. I needed to manually restore context to the space cluster's kubeconfig via kubectl config set-context.
    • We need to fix navigation here.
  3. Commands should assert the relevant level and fail early for better UX. When I am on a controlplane context, up ctp list gives me up: error: controlplane.listCmd.Run(): error getting control planes: no matches for kind "ControlPlane" in version "spaces.upbound.io/v1beta1" , I'd expect that this should inform me to navigate out of the controlplane level and do not try to issue the command.