CLI Authentication #252
Merged
CLI Authentication #252
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This would be a good one for @Bobbins228 to review as well, as he's pretty familiar with the new auth updates (being the one who wrote them 😁) |
Bobbins228
approved these changes
Jul 28, 2023
Maxusmusti
requested changes
Jul 28, 2023
There was a problem hiding this comment.
Put this in slack, but adding this here as well so nobody accidentally merges without seeing:
Looks good, just a couple of things:
- When things are saved in "auth", that's an actual file getting created and stored, right? If so, be careful about using a relative path, as people may login, move to a different dir, then be confused as to why they are logged out. We may instead want this file to be in a set
~/.codeflare/auth - In your unit tests, it may be worth checking that the contents of the
authfile are as you expect as well - Just checking, where is the
load_auth()function going to be used, is it going to be at the start of every other CLI function? - I'm assuming this is just for token auth? That makes sense, as users can otherwise just do their own
export KUBECONFIG=...if they need to change default kubeconfig path. I would just test and make sure that that option still works as expected
Maxusmusti
pushed a commit
that referenced
this pull request
Jul 31, 2023
* Add: create_api_client_config helper function for the SDK * Add: login function for CLI * Change: change options and help for login function * Create: logout function * Test: add unit tests for login and logout functions * add: additional error handling and change layout slightly * test: add unit test for load_auth * change: make tls skip false by default * add: make authentication go into .codeflare * test: add unit tests for checking validity of auth file and split login/logout tests
carsonmh
added a commit
that referenced
this pull request
Aug 11, 2023
* Add: create_api_client_config helper function for the SDK * Add: login function for CLI * Change: change options and help for login function * Create: logout function * Test: add unit tests for login and logout functions * add: additional error handling and change layout slightly * test: add unit test for load_auth * change: make tls skip false by default * add: make authentication go into .codeflare * test: add unit tests for checking validity of auth file and split login/logout tests
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue link
closes #217
What changes have been made
Made login and logout functions in the CLI, allowing the user to authenticate into their Kubernetes cluster using token + server. Also added
load_authfunction which loads the authentication configuration so the SDK can use it.Verification steps
codeflare loginandcodeflare logoutare the two functions to be tested. Can be run in the terminal after building SDK withpip install -e .pytest -v tests/unit_test.pyorpython -m pytest tests/unit_test.pyChecks