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.

Sign up for GitHub

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

Jump to bottom

rpk profile changes #17038

Merged
merged 16 commits into from
Mar 15, 2024
Merged

rpk profile changes #17038

merged 16 commits into from
Mar 15, 2024

Conversation

twmb
Copy link
Contributor

@twmb twmb commented Mar 13, 2024 •
edited
Loading

  • Deprecates multiple rpk cloud auth commands, which were confusing and are now unnecessary
  • Improves profile auth management so that it is easier to log into multiple orgs
  • On startup, prompts to remove old cloud profiles and auths that are no longer valid

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v23.3.x
  • v23.2.x

Release Notes

Improvements

twmb added 2 commits March 11, 2024 02:51
@twmb
rpk: profile & cloud login rework
117c58a 
SMALL
* Adds "rpk" to the early-check-exit error message; if a person has some
  rpk command in their bashrc, it is useful to know why rpk startup is
  failing

LARGE
* Reworks logins and profiles to be more explicit about the
  organizations being used
@twmb
rpk: improve profile create, add cloud cluster use
ece9c0f
twmb
twmb commented Mar 13, 2024
View reviewed changes
src/go/rpk/pkg/config/params.go Outdated Show resolved Hide resolved
twmb
twmb commented Mar 13, 2024
View reviewed changes
src/go/rpk/pkg/oauth/load.go Outdated Show resolved Hide resolved
twmb
twmb commented Mar 13, 2024
View reviewed changes
src/go/rpk/pkg/oauth/load.go Outdated Show resolved Hide resolved
@vbotbuildovich
Copy link
Collaborator

vbotbuildovich commented Mar 13, 2024 •
edited
Loading

new failures in https://buildkite.com/redpanda/redpanda/builds/46142#018e39c9-e3d2-41ff-a9c2-da33fa0bc8c5:

"rptest.tests.rpk_profile_test.RpkProfileTest.test_use_profile"

new failures in https://buildkite.com/redpanda/redpanda/builds/46142#018e39c9-e3cc-447b-a324-593a0c44835b:

"rptest.tests.rpk_profile_test.RpkProfileTest.test_e2e_profile"

new failures in https://buildkite.com/redpanda/redpanda/builds/46142#018e39d1-441b-4063-bcd5-eda1b0520139:

"rptest.tests.rpk_profile_test.RpkProfileTest.test_use_profile"

new failures in https://buildkite.com/redpanda/redpanda/builds/46142#018e39d1-4417-4a16-8dc8-86c3e640a718:

"rptest.tests.rpk_profile_test.RpkProfileTest.test_e2e_profile"

new failures in https://buildkite.com/redpanda/redpanda/builds/46167#018e3c05-52f7-4382-b32e-5617671f6fee:

"rptest.tests.rpk_profile_test.RpkProfileTest.test_use_profile"

new failures in https://buildkite.com/redpanda/redpanda/builds/46167#018e3c05-52f4-4df6-b972-47309d1f0436:

"rptest.tests.rpk_profile_test.RpkProfileTest.test_e2e_profile"

new failures in https://buildkite.com/redpanda/redpanda/builds/46167#018e3c16-01d3-4626-9eb0-14988d9cb4ae:

"rptest.tests.rpk_profile_test.RpkProfileTest.test_use_profile"

new failures in https://buildkite.com/redpanda/redpanda/builds/46167#018e3c16-01d0-4fb1-863a-5c91602c968a:

"rptest.tests.rpk_profile_test.RpkProfileTest.test_e2e_profile"

vladoschreiner
vladoschreiner reviewed Mar 14, 2024
View reviewed changes
src/go/rpk/pkg/cli/cloud/login.go Outdated

// When you have no profile, or you have one that is not selected.
if p == nil || len(y.Profiles) == 0 {
fmt.Println(`To create an rpk profile to talk to an existing cloud cluster, use 'rpk cloud cluster use'.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the changes we discussed in person (change to the language we designed).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This now only prints if you use --no-profile -- new messaging is better, and I think this is worth keeping for the --no-profile flag

vladoschreiner
vladoschreiner reviewed Mar 14, 2024
View reviewed changes
src/go/rpk/pkg/cli/cloud/login.go Outdated
// having a different org's auth.
if p != nil && clearedProfile {
priorAuth := p.ActualAuth()
fmt.Printf(`rpk swapped away from your prior profile %q which authenticated with organization %q (%s).
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reduce verbosity, more directive language:

Login token change detected

Select cloud cluster to talk to:
    rpk cloud cluster use

rpk will talk to a localhost:9092 cluster otherwise

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually this one, I'd like to keep the current messaging (or the details of it) -- a lot of the confusion people were running into was what happened when they switched orgs, and I want to make sure it's really clear to end users that their auth is now talking to a different org.

@twmb twmb added this to the v23.3.8 milestone Mar 14, 2024
twmb added 2 commits March 14, 2024 13:51
@twmb
rpk: profile updates for UAT
2a6609c
@twmb
rpk profile: fixup auth
35635ef 
Kind is now no longer computed, making some things more robust and
easier to reason about.
twmb added 3 commits March 14, 2024 17:11
@twmb
rpk: use virtual auth when creating profiles
9d55535 
The virtual auth may contain client IDs or secrets that are not mirrored
in the actual auth.
@twmb
rpk: take into account auth kind to clear profiles
9c6ec38 
If we have profile A from auth SSO, we cannot keep profile A if we
subsequently log in with client credentials. Even though the
organizations are the same, the auth is different, and we need a new
profile tied to the new auth.
@twmb
rpk: update tests for prior commit profile changes
9501251 
This also removes `omitempty` from a bunch of fields, which should make
it clearer in `rpk profile edit` what potential fields exist.
@twmb
Copy link
Contributor Author

twmb commented Mar 15, 2024

Closes #12802, except for the documented yaml -- this can be a separate issue.

r-vasquez
r-vasquez reviewed Mar 15, 2024
View reviewed changes
src/go/rpk/pkg/cli/cloud/auth/delete.go Outdated Show resolved Hide resolved
src/go/rpk/pkg/cli/cloud/auth/delete.go Outdated Show resolved Hide resolved
src/go/rpk/pkg/cli/cloud/auth/edit.go Outdated Show resolved Hide resolved
src/go/rpk/pkg/cli/cloud/auth/use.go Outdated Show resolved Hide resolved
src/go/rpk/pkg/cli/cloud/cluster/select.go Show resolved Hide resolved
src/go/rpk/pkg/cli/profile/edit.go Outdated Show resolved Hide resolved
src/go/rpk/pkg/config/rpk_yaml_test.go Outdated Show resolved Hide resolved
r-vasquez
r-vasquez previously approved these changes Mar 15, 2024
View reviewed changes
Copy link
Contributor

@r-vasquez r-vasquez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one final comment

src/go/rpk/pkg/config/rpk_yaml.go Show resolved Hide resolved
@twmb
rpk yaml: omitempty on empty CloudCluster's
eb31bb0 
This makes the yaml look a bit nicer. Also updates the yaml sha,
sticking with v3 since v3 is introduced in this PR.
r-vasquez
r-vasquez approved these changes Mar 15, 2024
View reviewed changes
Copy link
Contributor

@r-vasquez r-vasquez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@twmb
Copy link
Contributor Author

twmb commented Mar 15, 2024

Merging -- Snyk is unrelated

@twmb twmb merged commit d383899 into redpanda-data:dev Mar 15, 2024
20 of 22 checks passed
@twmb twmb deleted the profile branch March 15, 2024 06:44
@vbotbuildovich
Copy link
Collaborator

/backport v23.3.x

@vbotbuildovich
Copy link
Collaborator

Failed to create a backport PR to v23.3.x branch. I tried:

git remote add upstream https://github.com/redpanda-data/redpanda.git
git fetch --all
git checkout -b backport-pr-17038-v23.3.x-972 remotes/upstream/v23.3.x
git cherry-pick -x 117c58a8bea4dcbee21cbf4641f03c6b7868c84d ece9c0f3e45d43122c21a13315bba8892731895f 34626165c8a29d97d7c536d749540a128a8664e7 ab3103bf0ff714b89d91d81c39564e439fb39f6b 7371a47291d348484b2af3529b3ecffde8bd34a9 0e45b2d6712abf29a0f73c0395130132a0534abd 2a6609cb18d5cde1df1c4ca4b88aa8914aaba365 35635ef27d31dbad7c6ceace152f82fdc8979f48 9d555354b5003067c34b80677e1cf58b085a1b52 9c6ec387df6613d48bca7ea1089b78ee370e118e 950125149c2c1605007df43f244d717495f5afb3 a72ad92cd5dcb122e187ec240d4c44d8f00d9d1a 94e8dc031497f4bb405fe4ac4d1212e51cd7b5d5 aaa96472d22043eedafb9ec056a9dfe411dce70d 967ccf988d4000955ed5d6175208f04476890da1 eb31bb0334528bc0d3f8a092023c5ce0ec4b3e0f

Workflow run logs.

@vbotbuildovich vbotbuildovich mentioned this pull request Mar 15, 2024
Closed
twmb added a commit to twmb/redpanda that referenced this pull request Mar 15, 2024
@twmb
rpk: profile updates for redpanda-data#17038 code review
dbbcf53 
(cherry picked from commit 3462616)
@twmb twmb mentioned this pull request Mar 15, 2024
Merged
6 tasks
twmb added a commit to twmb/redpanda that referenced this pull request Mar 15, 2024
@twmb
rpk: profile updates for redpanda-data#17038 code review
c09aa46 
(cherry picked from commit 3462616)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vladoschreiner vladoschreiner vladoschreiner left review comments

@r-vasquez r-vasquez r-vasquez approved these changes

@gene-redpanda gene-redpanda Awaiting requested review from gene-redpanda

Assignees
No one assigned
Labels
area/rpk
Projects
None yet
Milestone
v23.3.8
Development

Successfully merging this pull request may close these issues.
4 participants
@twmb @vbotbuildovich @vladoschreiner @r-vasquez