-
Notifications
You must be signed in to change notification settings - Fork 44
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
Adding security enabled integration tests #400
Conversation
Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #400 +/- ##
============================================
- Coverage 72.14% 71.95% -0.20%
Complexity 613 613
============================================
Files 79 79
Lines 3070 3081 +11
Branches 238 238
============================================
+ Hits 2215 2217 +2
- Misses 751 760 +9
Partials 104 104 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
Seems the client used to determine the security plugin is present in the cluster is refusing to connect, which is preventing our security integration tests to run :
|
Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
…red only for security y enabled clusters Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[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.
Looks good overall with few comments
src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java
Show resolved
Hide resolved
Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java
Outdated
Show resolved
Hide resolved
…low bug Signed-off-by: Joshua Palis <[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.
Looks good to me unless you are planning to add more tests in the same PR
src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java
Show resolved
Hide resolved
Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
…ces to security plugin configuration Signed-off-by: Joshua Palis <[email protected]>
The backport to
To backport manually, run these commands in your terminal: # Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/flow-framework/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/flow-framework/backport-2.x
# Create a new branch
git switch --create backport/backport-400-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 74f42ba712bfc008ddc21ca18402f4dde541fd1a
# Push it to GitHub
git push --set-upstream origin backport/backport-400-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/flow-framework/backport-2.x Then, create a pull request where the |
* Adding intiial security integration tests, addin test security workflow Signed-off-by: Joshua Palis <[email protected]> * updating set up to v4 Signed-off-by: Joshua Palis <[email protected]> * Fixing run docker image task Signed-off-by: Joshua Palis <[email protected]> * Fixing pull and run docket Signed-off-by: Joshua Palis <[email protected]> * Fixing pull and run docket Signed-off-by: Joshua Palis <[email protected]> * Testing integ test if security is not available Signed-off-by: Joshua Palis <[email protected]> * Removing non-security integ test from workflow Signed-off-by: Joshua Palis <[email protected]> * test Signed-off-by: Joshua Palis <[email protected]> * test Signed-off-by: Joshua Palis <[email protected]> * Removing docker -ps Signed-off-by: Joshua Palis <[email protected]> * Pulling in secuirty as a zipArchive dependency, installed and configured only for security y enabled clusters Signed-off-by: Joshua Palis <[email protected]> * fixing ci Signed-off-by: Joshua Palis <[email protected]> * using v1 Signed-off-by: Joshua Palis <[email protected]> * Addressing PR comments, using security.emabled system property instead Signed-off-by: Joshua Palis <[email protected]> * Adding remaining read access role tests Signed-off-by: Joshua Palis <[email protected]> * spotless Signed-off-by: Joshua Palis <[email protected]> * Addressing PR comments, adding full access tests, fixing create workflow bug Signed-off-by: Joshua Palis <[email protected]> * Added more APIs to full access client test Signed-off-by: Joshua Palis <[email protected]> * updating DEVELOPER_GUIDE Signed-off-by: Joshua Palis <[email protected]> * Updating developer guide, adding back ML Commons security system indices to security plugin configuration Signed-off-by: Joshua Palis <[email protected]> --------- Signed-off-by: Joshua Palis <[email protected]>
Adding security enabled integration tests (#400) * Adding intiial security integration tests, addin test security workflow * updating set up to v4 * Fixing run docker image task * Fixing pull and run docket * Fixing pull and run docket * Testing integ test if security is not available * Removing non-security integ test from workflow * test * test * Removing docker -ps * Pulling in secuirty as a zipArchive dependency, installed and configured only for security y enabled clusters * fixing ci * using v1 * Addressing PR comments, using security.emabled system property instead * Adding remaining read access role tests * spotless * Addressing PR comments, adding full access tests, fixing create workflow bug * Added more APIs to full access client test * updating DEVELOPER_GUIDE * Updating developer guide, adding back ML Commons security system indices to security plugin configuration --------- Signed-off-by: Joshua Palis <[email protected]>
Description
This PR aims to add security enabled integration tests to the CI in order to test various cluster permissions based on the opendistro security roles added by opensearch-project/security#3851. The tests themselves are relatively simple, we create two users, one with a
flow_framework_full_access
role, and one with aflow_framework_read_access
role to determine if these users can hit our APIs.The
build.gradle
has been modified to automatically configure an integration test cluster with or without the security plugin installed depending on the provided system properties. The following details the commands to run an integration test cluster with or without security :By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.