Revert "Integrate k-NN functionality with security plugin"

[jmazanec15]

Description

This reverts commit 6ded82d.

This change is necessary because security integration tests are failing for 2.6 due to k-NN's system index not being accessible to perform writes to it. The fix will take some changes on the k-NN side, so for now, we will revert to unblock release.

Issues Resolved

#2478

Testing

[Please provide details of testing done: unit testing, integration testing and manual testing]

Check List

• New functionality includes testing
• New functionality has been documented
• Commits are signed per the DCO using --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.

[peternied]

@jmazanec15 Have you been able to run the tests with the revert of this change applied. How do we know this change is responsible for the issues in the integration tests - could you provide those details?

[VijayanB]

The integration test was working in 2.5 before this change was back ported . Hence, we would like to revert this. @jmazanec15 can you confirm?

[peternied]

FYI - If we have confirmation I'll immediately revert in the 2.6 branch, and close this pull request.

[codecov-commenter]

Codecov Report

Merging #2480 (1aeb37a) into main (2df8acd) will decrease coverage by 0.01%.
The diff coverage is n/a.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@             Coverage Diff              @@
##               main    #2480      +/-   ##
============================================
- Coverage     61.19%   61.19%   -0.01%     
+ Complexity     3325     3323       -2     
============================================
  Files           260      260              
  Lines         18494    18494              
  Branches       3268     3268              
============================================
- Hits          11318    11317       -1     
  Misses         5578     5578              
- Partials       1598     1599       +1     

Impacted Files	Coverage Δ
...search/security/transport/SecurityInterceptor.java	75.38% <0.00%> (-0.77%)	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[jmazanec15]

@peternied Sure, the problem is that in this change, the ".opensearch-knn-models" index is marked as a system index in security demo. With this, there are protections that come into play. We were seeing the following error:

[2023-02-23T22:29:33,608][ERROR][o.o.k.t.TrainingJobRunner] [] Unable to initialize model serialization: no permissions for [] and User [name=admin, backend_roles=[admin], requestedTenant=null]

This was happening because our training api internally will make a request to index a document into our system index: https://github.com/opensearch-project/k-NN/blob/main/src/main/java/org/opensearch/knn/indices/ModelDao.java#L341. However, this is failing because only super admin users are able to add documents into system indices - here we are just using the normal client, not admin. I am not sure how to fix this yet, as it is not possible to index through the admin client. Im discussing with the AD team, but that might take some time.

Im confident this change won't cause any other problems by removing system index status. Without this protection, this class wont get invoked: https://github.com/opensearch-project/security/blob/main/src/main/java/org/opensearch/security/privileges/SecurityIndexAccessEvaluator.java#L124, avoiding the issue.

We were also getting an error deleting system indices, but that can be resolved by setting up the required certs similar to AD: https://github.com/opensearch-project/anomaly-detection/tree/main/src/test/resources/security.

[peternied]

The revert has been pushed to the 2.6 branch 766cf3f

Thanks, @jmazanec15 and @VijayanB - please work with the release team to get the updated build and confirm the integration tests are fixed.

For adhoc testing, we've published the updated plugin, download

[jmazanec15]

@peternied Thanks!