Skip to content

Commit

Permalink
rpk: profile updates for redpanda-data#17038 code review
Browse files Browse the repository at this point in the history
(cherry picked from commit 3462616)
  • Loading branch information
twmb committed Mar 15, 2024
1 parent d1bdb3e commit c09aa46
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 55 deletions.
2 changes: 1 addition & 1 deletion src/go/rpk/pkg/cli/cloud/auth/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func newCreateCommand(fs afero.Fs, p *config.Params) *cobra.Command {
Short: "Create an rpk cloud auth",
Hidden: true,
Run: func(*cobra.Command, []string) {
fmt.Println("'rpk cloud auth create' is deprecated as a no-op; use 'rpk cloud login --new-credentials' instead.")
fmt.Println("'rpk cloud auth create' is deprecated as a no-op; use 'rpk cloud login --reload' instead.")
},
}
var slice []string
Expand Down
101 changes: 68 additions & 33 deletions src/go/rpk/pkg/cli/profile/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ Either:
Or:
rpk profile create $another_name --your-flags-here
`, ee.Name)
os.Exit(1)
}
out.MaybeDieErr(err)
},
Expand Down Expand Up @@ -196,7 +197,7 @@ func CreateFlow(
p = &o.Profile
if name == "" {
name = RpkCloudProfileName
if old := yAct.Profile(p.Name); old != nil {
if old := yAct.Profile(name); old != nil {
if !old.FromCloud {
return &ProfileExistsError{name}
}
Expand Down Expand Up @@ -331,38 +332,25 @@ func createCloudProfile(ctx context.Context, y *config.RpkYaml, cfg *config.Conf
return PromptCloudClusterProfile(ctx, cl)
}

var clusterID string
var (
clusterID string
triedNameLookup bool
forceNameLookup bool
)
nameLookup:
_, err = xid.FromString(clusterIDOrName)
if err != nil {
_, nss, vcs, cs, err := cl.OrgNamespacesClusters(ctx)
if err != nil || forceNameLookup {
clusterID, err = clusterNameToID(ctx, cl, clusterIDOrName)
if err != nil {
return CloudClusterOutputs{}, fmt.Errorf("unable to request organization, namespace, or cluster details: %w", err)
}
candidates := findNamedCluster(clusterIDOrName, nss, vcs, cs)

switch len(candidates) {
case 0:
return CloudClusterOutputs{}, fmt.Errorf("no cluster found with name %q", clusterIDOrName)
case 1:
for _, nc := range candidates {
if nc.IsVCluster {
clusterID = nc.VCluster.ID
} else {
clusterID = nc.Cluster.ID
}
break
}
default:
ncs := combineClusterNames(nss, vcs, cs)
names := ncs.names()

idx, err := out.PickIndex(names, "Multiple clusters found with the requested name, please select one:")
if err != nil {
return CloudClusterOutputs{}, err
if forceNameLookup {
// It is possible that an API call failed, but odds are *at this point*
// that we can query the API successfully (as we have already done so
// multiple times), and the problem is the name does not exist.
return CloudClusterOutputs{}, fmt.Errorf("unable to find a cluster ID nor a cluster name for %q", clusterIDOrName)
}
clusterID = ncs[idx].clusterID()

return CloudClusterOutputs{}, err
}
triedNameLookup = true
} else {
clusterID = clusterIDOrName
}
Expand All @@ -371,33 +359,77 @@ func createCloudProfile(ctx context.Context, y *config.RpkYaml, cfg *config.Conf
if err != nil { // if we fail for a vcluster, we try again for a normal cluster
c, err := cl.Cluster(ctx, clusterID)
if err != nil {
// If the input cluster looks like an xid, we try
// parsing it as a cluster ID. If the xid lookup fails,
// it is POSSIBLE that the cluster name itself looks
// like an xid. We retry from the top forcing a name
// lookup; if that also fails, we return early above.
if !forceNameLookup && !triedNameLookup {
forceNameLookup = true
goto nameLookup
}
return CloudClusterOutputs{}, fmt.Errorf("unable to request details for cluster %q: %w", clusterID, err)
}
return fromCloudCluster(c), nil
}
return fromVirtualCluster(vc), nil
}

func clusterNameToID(ctx context.Context, cl *cloudapi.Client, name string) (string, error) {
_, nss, vcs, cs, err := cl.OrgNamespacesClusters(ctx)
if err != nil {
return "", fmt.Errorf("unable to request organization, namespace, or cluster details: %w", err)
}
candidates := findNamedCluster(name, nss, vcs, cs)

switch len(candidates) {
case 0:
return "", fmt.Errorf("no cluster found with name %q", name)
case 1:
for _, nc := range candidates {
if nc.IsVCluster {
return nc.VCluster.ID, nil
} else {
return nc.Cluster.ID, nil
}
}
panic("unreachable")
default:
ncs := combineClusterNames(nss, vcs, cs)
names := ncs.names()

idx, err := out.PickIndex(names, "Multiple clusters found with the requested name, please select one:")
if err != nil {
return "", err
}
return ncs[idx].clusterID(), nil
}
}

// Iterates across vcs and cs and returns all clusters that match the given
// name.
func findNamedCluster(name string, nss []cloudapi.Namespace, vcs []cloudapi.VirtualCluster, cs []cloudapi.Cluster) map[string]cloudapi.NamespacedCluster {
ret := make(map[string]cloudapi.NamespacedCluster)
namespaceIDs := make(map[string]cloudapi.Namespace, len(nss))
for _, ns := range nss {
namespaceIDs[ns.ID] = ns
}
for _, vc := range vcs {
if vc.Name != name {
if name != vc.Name && name != fmt.Sprintf("%s/%s", namespaceIDs[vc.NamespaceUUID].Name, vc.Name) {
continue
}
ns := cloudapi.FindNamespace(vc.NamespaceUUID, nss)
ns := namespaceIDs[vc.NamespaceUUID]
ret[vc.Name] = cloudapi.NamespacedCluster{
Namespace: ns,
VCluster: vc,
IsVCluster: true,
}
}
for _, c := range cs {
if c.Name != name {
if name != c.Name && name != fmt.Sprintf("%s/%s", namespaceIDs[c.NamespaceUUID].Name, c.Name) {
continue
}
ns := cloudapi.FindNamespace(c.NamespaceUUID, nss)
ns := namespaceIDs[c.NamespaceUUID]
ret[c.Name] = cloudapi.NamespacedCluster{
Namespace: ns,
Cluster: c,
Expand Down Expand Up @@ -608,6 +640,9 @@ func combineClusterNames(nss cloudapi.Namespaces, vcs []cloudapi.VirtualCluster,
var vNameAndCs []nameAndCluster
for _, vc := range vcs {
vc := vc
if strings.ToLower(vc.State) != cloudapi.ClusterStateReady {
continue
}
vNameAndCs = append(vNameAndCs, nameAndCluster{
name: fmt.Sprintf("%s/%s", nsIDToName[vc.NamespaceUUID], vc.Name),
vc: &vc,
Expand Down
2 changes: 0 additions & 2 deletions src/go/rpk/pkg/config/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -365,8 +365,6 @@ var xflags = map[string]xflag{
p := y.Profile(y.CurrentProfile)
auth := p.VirtualAuth()
if auth == nil {
// If there is no actual current auth, we the virtual
// yaml has a "default" CloudCurrentAuth with no org.
auth = y.CurrentAuth()
}
auth.ClientID = v
Expand Down
17 changes: 2 additions & 15 deletions src/go/rpk/pkg/config/rpk_yaml.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,19 +214,6 @@ func (y *RpkYaml) LookupAuth(org, kind string) *RpkCloudAuth {
return nil
}

// LookupAuthByName returns all auths with the given name.
// There will be multiple matching auths if a person is authenticated
// to multiple orgs that are named the same.
func (y *RpkYaml) LookupAuthByName(name string) []*RpkCloudAuth {
var matches []*RpkCloudAuth
for i := range y.CloudAuths {
if y.CloudAuths[i].Name == name {
matches = append(matches, &y.CloudAuths[i])
}
}
return matches
}

// PushNewAuth pushes an auth to the front and sets it as the current auth.
func (y *RpkYaml) PushNewAuth(a RpkCloudAuth) {
y.CloudAuths = append([]RpkCloudAuth{a}, y.CloudAuths...)
Expand Down Expand Up @@ -367,15 +354,15 @@ func (a *RpkCloudAuth) HasClientCredentials() bool {
}

// Equals returns if the two cloud auths are the same, which is true
// if the name and org ID match (ignoring all other values).
// if the name matches (the name embeds the org name, ID, and auth kind).
func (a *RpkCloudAuth) Equals(other *RpkCloudAuth) bool {
if a == nil && other == nil {
return true
}
if a == nil || other == nil {
return false
}
return a.Name == other.Name && a.OrgID == other.OrgID
return a.Name == other.Name
}

// Returns if the raw config is the same as the one in memory.
Expand Down
5 changes: 1 addition & 4 deletions src/go/rpk/pkg/oauth/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,7 @@ func LoadFlow(ctx context.Context, fs afero.Fs, cfg *config.Config, cl Client, n
// If this is a new token and we have a cloud profile, we need to
// lookup the organization. The SSO flow may have logged into a
// different org than the current auth and if so, we need to clear
// the current profile. We do this same logic for client credentials
// this will be a no-op basically.
// the current profile.
if pAuthAct := yAct.Profile(yAct.CurrentProfile).ActualAuth(); isNewToken && pAuthAct != nil {
org, orgErr := getOrg()
if orgErr != nil {
Expand Down Expand Up @@ -292,8 +291,6 @@ func LoadFlow(ctx context.Context, fs afero.Fs, cfg *config.Config, cl Client, n
yVir.CurrentCloudAuthOrgID = org.ID
yAct.CurrentCloudAuthOrgID = org.ID

fmt.Println("updating currents, orgID", org.ID, "kind", authKind)

yVir.CurrentCloudAuthKind = string(authVir.AnyKind())
yAct.CurrentCloudAuthKind = string(authVir.AnyKind()) // we use the virtual kind here -- clientID is updated below

Expand Down

0 comments on commit c09aa46

Please sign in to comment.