-
Notifications
You must be signed in to change notification settings - Fork 75
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
Fix Security Tests After Changes to Permissions Requirements #1308
Conversation
This PR addresses errors in security tests caused by recent changes in opensearch-project/security#4719. Previously, users needed both AD full access and source index permissions to fully utilize anomaly detection (AD). AD full access has already included all alias and mapping permissions. it was inconsistent not to include index search permission, which would otherwise force users to create an additional role. The change in the referenced PR aimed to simplify user management. Due to this change, existing security tests that relied on a user having AD full access but lacking data search permission would no longer trigger the expected search permission exception. This PR addresses that issue by creating a new user role with only AD read permission (note we didn't change ad read access permission in the referenced PR) and without source index search permission, ensuring the tests correctly validate the lack of search permissions. Testing Done: * Verified that previously failing security tests now pass Signed-off-by: Kaituo Li <[email protected]>
I see we are getting NoShardAvailableActionException again on bwc, is the same as we saw before at some point? |
CI check for BWC is failing |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1308 +/- ##
============================================
+ Coverage 71.83% 77.44% +5.61%
- Complexity 4898 5437 +539
============================================
Files 518 533 +15
Lines 22879 23324 +445
Branches 2245 2311 +66
============================================
+ Hits 16434 18064 +1630
+ Misses 5410 4199 -1211
- Partials 1035 1061 +26
Flags with carried forward coverage won't be shown. Click here to find out more. |
This PR addresses errors in security tests caused by recent changes in opensearch-project/security#4719. Previously, users needed both AD full access and source index permissions to fully utilize anomaly detection (AD). AD full access has already included all alias and mapping permissions. it was inconsistent not to include index search permission, which would otherwise force users to create an additional role. The change in the referenced PR aimed to simplify user management. Due to this change, existing security tests that relied on a user having AD full access but lacking data search permission would no longer trigger the expected search permission exception. This PR addresses that issue by creating a new user role with only AD read permission (note we didn't change ad read access permission in the referenced PR) and without source index search permission, ensuring the tests correctly validate the lack of search permissions. Testing Done: * Verified that previously failing security tests now pass Signed-off-by: Kaituo Li <[email protected]> (cherry picked from commit 0aebc6d) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
This PR addresses errors in security tests caused by recent changes in opensearch-project/security#4719. Previously, users needed both AD full access and source index permissions to fully utilize anomaly detection (AD). AD full access has already included all alias and mapping permissions. it was inconsistent not to include index search permission, which would otherwise force users to create an additional role. The change in the referenced PR aimed to simplify user management. Due to this change, existing security tests that relied on a user having AD full access but lacking data search permission would no longer trigger the expected search permission exception. This PR addresses that issue by creating a new user role with only AD read permission (note we didn't change ad read access permission in the referenced PR) and without source index search permission, ensuring the tests correctly validate the lack of search permissions. Testing Done: * Verified that previously failing security tests now pass Signed-off-by: Kaituo Li <[email protected]> (cherry picked from commit 0aebc6d) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@amitgalitz @vibrantvarun I don't recall. Should be unrelated to security test change. Will try to fix it when I see it again |
…1309) This PR addresses errors in security tests caused by recent changes in opensearch-project/security#4719. Previously, users needed both AD full access and source index permissions to fully utilize anomaly detection (AD). AD full access has already included all alias and mapping permissions. it was inconsistent not to include index search permission, which would otherwise force users to create an additional role. The change in the referenced PR aimed to simplify user management. Due to this change, existing security tests that relied on a user having AD full access but lacking data search permission would no longer trigger the expected search permission exception. This PR addresses that issue by creating a new user role with only AD read permission (note we didn't change ad read access permission in the referenced PR) and without source index search permission, ensuring the tests correctly validate the lack of search permissions. Testing Done: * Verified that previously failing security tests now pass (cherry picked from commit 0aebc6d) Signed-off-by: Kaituo Li <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…1310) This PR addresses errors in security tests caused by recent changes in opensearch-project/security#4719. Previously, users needed both AD full access and source index permissions to fully utilize anomaly detection (AD). AD full access has already included all alias and mapping permissions. it was inconsistent not to include index search permission, which would otherwise force users to create an additional role. The change in the referenced PR aimed to simplify user management. Due to this change, existing security tests that relied on a user having AD full access but lacking data search permission would no longer trigger the expected search permission exception. This PR addresses that issue by creating a new user role with only AD read permission (note we didn't change ad read access permission in the referenced PR) and without source index search permission, ensuring the tests correctly validate the lack of search permissions. Testing Done: * Verified that previously failing security tests now pass (cherry picked from commit 0aebc6d) Signed-off-by: Kaituo Li <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Description
This PR addresses errors in security tests caused by recent changes in opensearch-project/security#4719. Previously, users needed both AD full access and source index permissions to fully utilize anomaly detection (AD). AD full access has already included all alias and mapping permissions. it was inconsistent not to include index search permission, which would otherwise force users to create an additional role. The change in the referenced PR aimed to simplify user management.
Due to this change, existing security tests that relied on a user having AD full access but lacking data search permission would no longer trigger the expected search permission exception. This PR addresses that issue by creating a new user role with only AD read permission (note we didn't change ad read access permission in the referenced PR) and without source index search permission, ensuring the tests correctly validate the lack of search permissions.
Testing Done:
Related Issues
#1307
Check List
--signoff
.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.