Skip to content

Conversation

@pkazmierczak
Copy link
Contributor

@pkazmierczak pkazmierczak commented Mar 15, 2023

nomad login command does not need to know ACL Auth Method's type, since all method names are unique. I discovered this while fixing a bug in which login ignored the existence of default auth method and errored when users didn't provide a method name or type. I believe this bug comes from the fact that originally we intended to have default-per-type design, and later we decided it's better UX to have global-default auth methods.

Closes #16501.

@pkazmierczak pkazmierczak self-assigned this Mar 15, 2023
@pkazmierczak pkazmierczak added the backport/1.5.x backport to 1.5.x release line label Mar 15, 2023
@pkazmierczak pkazmierczak requested review from jrasell and lgfa29 March 15, 2023 13:27
@pkazmierczak pkazmierczak changed the title cli: fix a bug in the login command that ignores the default auth method cli: nomad login command should not require a -type flag and should respect default auth method Mar 15, 2023
Copy link
Member

@jrasell jrasell left a comment

Choose a reason for hiding this comment

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

LGTM, with a tiny grammar comment change. It might be worth pinging @mikenomitch to double check he is cool with the UX change.

It would also be worth preparing PR's for any tutorials that also need updating based on this, so they can be released at the right time.

Co-authored-by: James Rasell <[email protected]>
Copy link
Contributor

@mikenomitch mikenomitch left a comment

Choose a reason for hiding this comment

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

+1 to the UX change from me

@pkazmierczak pkazmierczak merged commit b95b105 into main Mar 17, 2023
@pkazmierczak pkazmierczak deleted the b-login-default branch March 17, 2023 18:14
pkazmierczak added a commit that referenced this pull request Mar 17, 2023
…espect default auth method (#16504)

nomad login command does not need to know ACL Auth Method's type, since all
method names are unique. 

Co-authored-by: James Rasell <[email protected]>
pkazmierczak added a commit that referenced this pull request Mar 17, 2023
@tgross tgross added this to the 1.5.2 milestone Mar 20, 2023
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 18, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

backport/1.5.x backport to 1.5.x release line

Projects

None yet

Development

Successfully merging this pull request may close these issues.

nomad login errors before determining the default auth method type

4 participants