Remove authentication requirement for launchpad init #7
Conversation
LucilleH
force-pushed
the
lucille--auth
branch
from
ef93864
to
2e92ed8
Compare
|
This seems less "remove auth from init" and more "provide a way for oss-launchpad users to upgrade into commercial launchpad". I thought the former was already done (I didn't notice we were calling auth.Identify(), but we can just remove it). The latter seems like a new feature, which is nice. |
I thought so too. Apparently not 🤷♀️ |
|
|
||
| clusterNames := []string{} | ||
| for _, c := range clusters { | ||
| clusterNames = append(clusterNames, c.GetName()) | ||
| } | ||
| // In case user wants to log in and use a jetpack managed cluster. | ||
| clusterNames = append(clusterNames, jetpackManagedCluster) |
There was a problem hiding this comment.
but, now we may have both jetpack-cloud and Jetpack managed cluster as options, which doesn't make sense.
There was a problem hiding this comment.
why don't we remove the Jetpack managed cluster option and then just indicate which clusters are jetpack-managed from the list of clusters we already have?
There was a problem hiding this comment.
Because if a commercial user is not logged in, we won't have the full set of clusters. By selecting Jetpack managed cluster we will drop them in the login flow, knowing that they are not OSS users.
There was a problem hiding this comment.
we won't have the full set of clusters
but we will, because they were (likely) previously merged into their kubeconfig
There was a problem hiding this comment.
If we do have a full set, then they can simply select it from the previous question. I'll give you a full demo
| return nil, errors.WithStack(err) | ||
| } | ||
| // Get all the cluster names again. | ||
| clusters, err := cmdOpts.ClusterProvider().GetAll(ctx) |
There was a problem hiding this comment.
hmm... i think this kind of breaks the oss experience.. which im actually OK with, but want us to be aware of the breakage.
In OSS, GetAll() will only ever return what's in your kubeconfig. so.. let's say you have clusters A and B. Then the prompt will be:
What cluster?
> A
B
Jetpack managed cluster
And then if you select Jetpack-managed cluster, we will try to show you jetpack-managed clusters, but there won't be any because we're on OSS.
There was a problem hiding this comment.
Right, in that case we skip the second question of displaying the jetpack-managed clusters, and ask them to run launchpad cluster create
padcli/command/initcmd.go
Outdated
| } | ||
|
|
||
| // If no jetpack managed cluster exist, we default the answer to the only option available. | ||
| if len(clusterNames) == 1 { |
There was a problem hiding this comment.
what if len(clusterNames) == 0?
Summary
This is the first step towards removing auth requirement for most commands, starting with init.
How was it tested?
cli-build
launchpad logout
launchpad init
Is this change backwards-compatible?
Yes