Skip to content

Commit

Permalink
rpk cloud login: change MakeAuthCurrent to **config.RpkCloudAuth
Browse files Browse the repository at this point in the history
When we made an auth current, we reordered the underlying auth slice
which is _not_ pointer based.

Any changes to the pointer fields _after_ reodering were changes to
an old slice that would no longer be saved.

Now, we MakeAuthCurrent with two pointer indirects --
The underlying pointer to the slice element is used for the invariant
check, did we find this auth.
We now update the pointer-to-the-pointer so that if we update any fields
_after_ MakeAuthCurrent, they're mirrored into the new slice.
  • Loading branch information
twmb committed Mar 18, 2024
1 parent fd5f462 commit 9dc5354
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 9 deletions.
2 changes: 1 addition & 1 deletion src/go/rpk/pkg/cli/cloud/auth/use.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ profile is kept.
}
}

y.MakeAuthCurrent(a)
y.MakeAuthCurrent(&a)
err = y.Write(fs)
out.MaybeDie(err, "unable to write rpk.yaml: %v", err)

Expand Down
12 changes: 7 additions & 5 deletions src/go/rpk/pkg/config/rpk_yaml.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,11 +241,12 @@ func (y *RpkYaml) PushNewAuth(a RpkCloudAuth) {
// MakeAuthCurrent finds the given auth, moves it to the front, and updates
// the current cloud auth fields. This pointer must exist, if it does not,
// this function panics.
func (y *RpkYaml) MakeAuthCurrent(a *RpkCloudAuth) {
reordered := []RpkCloudAuth{*a}
// This updates *a to point to the new address of the auth.
func (y *RpkYaml) MakeAuthCurrent(a **RpkCloudAuth) {
reordered := []RpkCloudAuth{**a}
var found bool
for i := range y.CloudAuths {
if &y.CloudAuths[i] == a {
if &y.CloudAuths[i] == *a {
found = true
continue
}
Expand All @@ -254,9 +255,10 @@ func (y *RpkYaml) MakeAuthCurrent(a *RpkCloudAuth) {
if !found {
panic("MakeAuthCurrent called with an auth that does not exist")
}
*a = &reordered[0]
y.CloudAuths = reordered
y.CurrentCloudAuthOrgID = a.OrgID
y.CurrentCloudAuthKind = a.Kind
y.CurrentCloudAuthOrgID = (*a).OrgID
y.CurrentCloudAuthKind = (*a).Kind
}

// DropAuth removes the given auth from the list of auths. If this was the
Expand Down
6 changes: 3 additions & 3 deletions src/go/rpk/pkg/oauth/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ func LoadFlow(ctx context.Context, fs afero.Fs, cfg *config.Config, cl Client, n
// This is case 1 or 2, the logic is identical. We start with
// some case 2 logic: we know the auth actually exists, so we
// enforce it is the current rpk.yaml auth.
yAct.MakeAuthCurrent(pAuthAct)
yAct.MakeAuthCurrent(&pAuthAct)

// Case 1 and 2: we check some invariants and then ensure the
// virtual rpk.yaml also has the same current auth.
Expand All @@ -185,7 +185,7 @@ func LoadFlow(ctx context.Context, fs afero.Fs, cfg *config.Config, cl Client, n
if !pAuthVir.Equals(pAuthAct) {
panic("params invariant: internal virtual auth name/org != actual name/org")
}
yVir.MakeAuthCurrent(pAuthVir)
yVir.MakeAuthCurrent(&pAuthVir)

// Finally, set authAct and authVir so they can be updated below.
authAct = pAuthAct
Expand Down Expand Up @@ -272,7 +272,7 @@ func LoadFlow(ctx context.Context, fs afero.Fs, cfg *config.Config, cl Client, n
a := &y.CloudAuths[i]
if a.OrgID == org.ID && a.Kind == authKind {
*newAuth = a
y.MakeAuthCurrent(*newAuth)
y.MakeAuthCurrent(newAuth)
return
}
}
Expand Down

0 comments on commit 9dc5354

Please sign in to comment.