-
Notifications
You must be signed in to change notification settings - Fork 57
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.
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
Add test script for authentication #1307
Conversation
Signed-off-by: Saad Khan <[email protected]>
4b448ab
to
9eeb3da
Compare
Signed-off-by: Saad Khan <[email protected]>
@@ -182,3 +182,13 @@ Else, you can change the workload name and namespace name in the test to match w | |||
|
|||
Note: The test will fail if it's run as is if there are no matching workloads that the test looks for. This test result can be ignored in case of a non-gpu workload | |||
|
|||
### Authentication Test: | |||
|
|||
Kruize 0.1.1 supports the authentication which provides the user an option to pass authentication details in the yaml for the service they are using. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be 0.2 instead of 0.1.1. Also describe the scenarios added here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -51,97 +33,149 @@ tokens=( | |||
["invalid"]="random-invalid-token-string" | |||
["empty"]="" | |||
) | |||
# Tests to validate authentication types in Kruize | |||
function authentication_tests() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you share the results run
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@khansaad - Expired token I think is same as invalid token here, we need to check how to simulate expired token. We cannot do this later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I think it will be good to invoke some code that makes use of the token along with checking the pod log, you can add it as an enhancement to the test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created #1386 to track this
Signed-off-by: Saad Khan <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
This PR adds a new script to test the authentication feature.
Fixes # (1286)
Type of change
How has this been tested?
Please describe the tests that were run to verify your changes and steps to reproduce. Please specify any test configuration required.
Test Configuration
Checklist 🎯
Additional information
Include any additional information such as links, test results, screenshots here