applying multi-tenancy in search [model, model group, agent, connector] #3433

dhrubo-os · 2025-01-25T18:31:12Z

Description

[Describe what this change achieves]

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

Check List

New functionality includes testing.
New functionality has been documented.
API changes companion pull request created.
Commits are signed per the DCO using --signoff.
Public documentation issue/PR created.

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.

Signed-off-by: Dhrubo Saha <[email protected]>

arjunkumargiri · 2025-01-28T06:56:48Z

common/src/main/java/org/opensearch/ml/common/transport/search/MLSearchActionRequest.java

+import lombok.Getter;
+
+@Getter
+public class MLSearchActionRequest extends SearchRequest {


Add java documentation

arjunkumargiri · 2025-01-28T06:59:37Z

plugin/src/main/java/org/opensearch/ml/action/connector/SearchConnectorTransportAction.java

@@ -96,4 +129,26 @@ private void search(SearchRequest request, ActionListener<SearchResponse> action
            actionListener.onFailure(e);
        }
    }
+
+    @VisibleForTesting
+    public static void wrapListenerToHandleConnectorIndexNotFound(Exception e, ActionListener<SearchResponse> listener) {


This method is currently not being used.

This method is being used in unit tests. And after sdkclient implementation we will use it here if we need too.

arjunkumargiri · 2025-01-28T07:00:00Z

plugin/src/main/java/org/opensearch/ml/action/handler/MLSearchHandler.java

+
+            // For multi-tenancy
+            if (tenantId != null) {
+                shouldQuery.should(QueryBuilders.termQuery(TENANT_ID_FIELD, tenantId));
+            }


thanks, removed.

arjunkumargiri · 2025-01-28T07:00:45Z

plugin/src/main/java/org/opensearch/ml/action/model_group/SearchModelGroupTransportAction.java

+                    queryBuilder.must(request.source().query());
+                }
+                // Add tenancy filter
+                queryBuilder.filter(QueryBuilders.termQuery(TENANT_ID_FIELD, tenantId)); // Replace 'tenant_id_field' with actual field name


Is the comment valid?

nope, removed.

Signed-off-by: Dhrubo Saha <[email protected]>

mingshl · 2025-01-28T19:54:07Z

common/src/main/java/org/opensearch/ml/common/transport/search/MLSearchActionRequest.java

+@Getter
+public class MLSearchActionRequest extends SearchRequest {
+    SearchRequest searchRequest;
+    String tenantId;


A new MLSearchActionRequest class was added to wrap the original SearchRequest and include a tenantId field.So I want to understand does it impact other functionality using SearchRequest?

Can you please give me an example which functionality are you referring to?

I think the order matters, when did the SearchRequest converts to MLSearchActionRequest? I am worry about the tools and search pipelines that are using the search requests. if the transform is after them, it should be fine, but if that's before tools and search pipelines, does it impact them?
for example

ml-commons/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/SearchIndexTool.java

Line 84 in 06a6021

private SearchRequest getSearchRequest(String index, String query) throws IOException {

ml-commons/search-processors/src/main/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQARequestProcessor.java

Line 51 in 06a6021

public SearchRequest processRequest(SearchRequest request) throws Exception {

mingshl · 2025-01-28T19:58:10Z

common/src/main/java/org/opensearch/ml/common/transport/search/MLSearchActionRequest.java

+                return new MLSearchActionRequest(input);
+            }
+        } catch (IOException e) {
+            throw new UncheckedIOException("failed to parse ActionRequest into MLSearchActionRequest", e);


In this exception handling, I think IllegalArgumentException is more proper in this case

The method is expecting an ActionRequest that can be converted to MLSearchActionRequest.
If the conversion fails, it's likely because the input ActionRequest is not of the expected type or format.
This is more of a logical/argument error than an I/O error.

I think if using IllegalArgumentException, it provides clearer feedback to the caller about what went wrong.

This is literally the same method we use for all other request classes. Example

opensearch-trigger-bot · 2025-01-28T20:10:57Z

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-3433-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 1422c7c6940143db4b92fba6fbb08a7acd10641b
# Push it to GitHub
git push --set-upstream origin backport/backport-3433-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-3433-to-2.x.

…r] (opensearch-project#3433) * applying multi-tenancy in search Signed-off-by: Dhrubo Saha <[email protected]> * addressed comments Signed-off-by: Dhrubo Saha <[email protected]> --------- Signed-off-by: Dhrubo Saha <[email protected]>

…agent, connector] (#3433) (#3443) * applying multi-tenancy in search [model, model group, agent, connector] (#3433) * applying multi-tenancy in search Signed-off-by: Dhrubo Saha <[email protected]> * addressed comments Signed-off-by: Dhrubo Saha <[email protected]> --------- Signed-off-by: Dhrubo Saha <[email protected]> * changing MLSearchActionRequest to an instance subclass of SearchActionRequest Signed-off-by: Dhrubo Saha <[email protected]> --------- Signed-off-by: Dhrubo Saha <[email protected]>

…agent, connector] (#3433) (#3443) * applying multi-tenancy in search [model, model group, agent, connector] (#3433) * applying multi-tenancy in search Signed-off-by: Dhrubo Saha <[email protected]> * addressed comments Signed-off-by: Dhrubo Saha <[email protected]> --------- Signed-off-by: Dhrubo Saha <[email protected]> * changing MLSearchActionRequest to an instance subclass of SearchActionRequest Signed-off-by: Dhrubo Saha <[email protected]> --------- Signed-off-by: Dhrubo Saha <[email protected]> (cherry picked from commit 34a7fb6)

…agent, connector] (#3433) (#3443) (#3469) * applying multi-tenancy in search [model, model group, agent, connector] (#3433) * applying multi-tenancy in search Signed-off-by: Dhrubo Saha <[email protected]> * addressed comments Signed-off-by: Dhrubo Saha <[email protected]> --------- Signed-off-by: Dhrubo Saha <[email protected]> * changing MLSearchActionRequest to an instance subclass of SearchActionRequest Signed-off-by: Dhrubo Saha <[email protected]> --------- Signed-off-by: Dhrubo Saha <[email protected]> (cherry picked from commit 34a7fb6) Co-authored-by: Dhrubo Saha <[email protected]>

dhrubo-os had a problem deploying to ml-commons-cicd-env January 25, 2025 18:32 — with GitHub Actions Failure

dhrubo-os force-pushed the multi_tenancy_search branch 2 times, most recently from 93b04e5 to 0b4eded Compare January 25, 2025 18:44

dhrubo-os had a problem deploying to ml-commons-cicd-env January 25, 2025 18:45 — with GitHub Actions Failure

dhrubo-os force-pushed the multi_tenancy_search branch from 0b4eded to 57f9afa Compare January 28, 2025 01:02

dhrubo-os had a problem deploying to ml-commons-cicd-env January 28, 2025 01:04 — with GitHub Actions Failure

dhrubo-os force-pushed the multi_tenancy_search branch from 57f9afa to 90745b2 Compare January 28, 2025 01:20

dhrubo-os had a problem deploying to ml-commons-cicd-env January 28, 2025 01:22 — with GitHub Actions Failure

dhrubo-os marked this pull request as ready for review January 28, 2025 01:51

dhrubo-os requested review from b4sjoo, mingshl, jngz-es, model-collapse, rbhavna, ylwu-amzn, zane-neo, Zhangxunmt, austintlee, HenryL27 and xinyual as code owners January 28, 2025 01:51

dhrubo-os force-pushed the multi_tenancy_search branch from 90745b2 to 7f32eb9 Compare January 28, 2025 02:09

dhrubo-os had a problem deploying to ml-commons-cicd-env January 28, 2025 02:11 — with GitHub Actions Failure

dhrubo-os force-pushed the multi_tenancy_search branch from 7f32eb9 to 22620bc Compare January 28, 2025 03:06

dhrubo-os had a problem deploying to ml-commons-cicd-env January 28, 2025 03:08 — with GitHub Actions Failure

dhrubo-os force-pushed the multi_tenancy_search branch from 22620bc to 88dc346 Compare January 28, 2025 04:26

applying multi-tenancy in search

9f653e8

Signed-off-by: Dhrubo Saha <[email protected]>

dhrubo-os force-pushed the multi_tenancy_search branch from 88dc346 to 9f653e8 Compare January 28, 2025 04:35

dhrubo-os temporarily deployed to ml-commons-cicd-env January 28, 2025 04:37 — with GitHub Actions Inactive

dhrubo-os had a problem deploying to ml-commons-cicd-env January 28, 2025 04:37 — with GitHub Actions Failure

dhrubo-os had a problem deploying to ml-commons-cicd-env January 28, 2025 06:09 — with GitHub Actions Failure

arjunkumargiri reviewed Jan 28, 2025

View reviewed changes

dhrubo-os had a problem deploying to ml-commons-cicd-env January 28, 2025 15:47 — with GitHub Actions Failure

dhrubo-os temporarily deployed to ml-commons-cicd-env January 28, 2025 17:39 — with GitHub Actions Inactive

addressed comments

87db43e

Signed-off-by: Dhrubo Saha <[email protected]>

dhrubo-os temporarily deployed to ml-commons-cicd-env January 28, 2025 17:46 — with GitHub Actions Inactive

dhrubo-os had a problem deploying to ml-commons-cicd-env January 28, 2025 17:46 — with GitHub Actions Failure

dhrubo-os had a problem deploying to ml-commons-cicd-env January 28, 2025 19:09 — with GitHub Actions Failure

arjunkumargiri approved these changes Jan 28, 2025

View reviewed changes

mingshl reviewed Jan 28, 2025

View reviewed changes

dhrubo-os had a problem deploying to ml-commons-cicd-env January 28, 2025 20:07 — with GitHub Actions Failure

jngz-es approved these changes Jan 28, 2025

View reviewed changes

ylwu-amzn approved these changes Jan 28, 2025

View reviewed changes

dhrubo-os merged commit 1422c7c into opensearch-project:main Jan 28, 2025
7 of 8 checks passed

dhrubo-os added the backport 2.x label Jan 28, 2025

dhrubo-os mentioned this pull request Jan 28, 2025

[BACKPORT 2.x] applying multi-tenancy in search [model, model group, agent, connector] (#3433) #3443

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

applying multi-tenancy in search [model, model group, agent, connector] #3433

applying multi-tenancy in search [model, model group, agent, connector] #3433

dhrubo-os commented Jan 25, 2025

arjunkumargiri Jan 28, 2025

dhrubo-os Jan 28, 2025

arjunkumargiri Jan 28, 2025

dhrubo-os Jan 28, 2025 •

edited

Loading

arjunkumargiri Jan 28, 2025

dhrubo-os Jan 28, 2025

arjunkumargiri Jan 28, 2025

dhrubo-os Jan 28, 2025

mingshl Jan 28, 2025

dhrubo-os Jan 28, 2025

mingshl Jan 28, 2025

mingshl Jan 28, 2025

dhrubo-os Jan 28, 2025

opensearch-trigger-bot bot commented Jan 28, 2025

applying multi-tenancy in search [model, model group, agent, connector] #3433

applying multi-tenancy in search [model, model group, agent, connector] #3433

Conversation

dhrubo-os commented Jan 25, 2025

Description

Related Issues

Check List

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dhrubo-os Jan 28, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

opensearch-trigger-bot bot commented Jan 28, 2025

dhrubo-os Jan 28, 2025 •

edited

Loading