Skip to content

Commit 7d61c3f

Browse files
DarshitChanpuradhrubo-os
DarshitChanpura
and
dhrubo-os
authored
Onboards to centralized resource access control mechanism for ml-model-group (opensearch-project#3715)
* Onboards to centralized resource access control mechanism for ml-model-group Signed-off-by: Darshit Chanpura <[email protected]> * Fixes sharing logic Signed-off-by: Darshit Chanpura <[email protected]> * Consumes feature flag exposed by SPI Signed-off-by: Darshit Chanpura <[email protected]> * Updates tests and dependency visibility Signed-off-by: Darshit Chanpura <[email protected]> * Fixes implmentation Signed-off-by: Darshit Chanpura <[email protected]> * Conforms to SPI changes Signed-off-by: Darshit Chanpura <[email protected]> * Conforms to SPI changes in security plugin Signed-off-by: Darshit Chanpura <[email protected]> * Reflects changes in SPI Signed-off-by: Darshit Chanpura <[email protected]> * Refactors to use centralized method in ModelAccessControlHelper Signed-off-by: Darshit Chanpura <[email protected]> * Corrects search handler to match against accessible model_group_ids Signed-off-by: Darshit Chanpura <[email protected]> * Fixes spotless Signed-off-by: Darshit Chanpura <[email protected]> * Fix missed reference Signed-off-by: Darshit Chanpura <[email protected]> * Fix compilation errors Signed-off-by: Darshit Chanpura <[email protected]> * Fix compileTestsJava Signed-off-by: Darshit Chanpura <[email protected]> * Adds existing feature-flag in conjunction with new feature flage Signed-off-by: Darshit Chanpura <[email protected]> * Fixes if condition Signed-off-by: Darshit Chanpura <[email protected]> * Updates default creation and update logic to support existing scenarios Signed-off-by: Darshit Chanpura <[email protected]> * Removes left over comment Signed-off-by: Darshit Chanpura <[email protected]> * Refactors to conform to standalone authz framework supplied by security plugin Signed-off-by: Darshit Chanpura <[email protected]> * Update model-group related requests to be doc requests Signed-off-by: Darshit Chanpura <[email protected]> * Refactors verifyAccess to pass action names Signed-off-by: Darshit Chanpura <[email protected]> * Re-add resource sharing client accessor Signed-off-by: Darshit Chanpura <[email protected]> * Refactors how resource-sharing client is accessed Signed-off-by: Darshit Chanpura <[email protected]> * Code cleanup Signed-off-by: Darshit Chanpura <[email protected]> * Code cleanup 2 Signed-off-by: Darshit Chanpura <[email protected]> * Code cleanup 3 Signed-off-by: Darshit Chanpura <[email protected]> * Fix test compilation error Signed-off-by: Darshit Chanpura <[email protected]> * Fix filter for model-groups on search request Signed-off-by: Darshit Chanpura <[email protected]> * Fix most of the tests Signed-off-by: Darshit Chanpura <[email protected]> * Fix search model transport action and tests Signed-off-by: Darshit Chanpura <[email protected]> * Fix test failure by passing correct listener Signed-off-by: Darshit Chanpura <[email protected]> * Refactors SearchModelGroupTransportActionTests to include new path and adds test for resource sharing extension Signed-off-by: Darshit Chanpura <[email protected]> * Fix spotless Signed-off-by: Darshit Chanpura <[email protected]> * Fix incorrect eval logic to check whether ml group index is present when search models Signed-off-by: Darshit Chanpura <[email protected]> * Adapts changes to make ResourceExtension consumed via @Inject Signed-off-by: Darshit Chanpura <[email protected]> * Fix UTs broken due to opensearch-project/common-utils#827 Signed-off-by: Darshit Chanpura <[email protected]> * Reverts "Adapts changes to make ResourceExtension consumed via @Inject" 8611f25 and adds setup for sec testing Signed-off-by: Darshit Chanpura <[email protected]> * Fixes extension over-ridden method Signed-off-by: Darshit Chanpura <[email protected]> * Fixes remaining broken UTs due to user object change Signed-off-by: Darshit Chanpura <[email protected]> * Adds plugin client, related permissions and resource-action-groups file Signed-off-by: Darshit Chanpura <[email protected]> * Updates resource-type and removes plugin client since it is not being used Signed-off-by: Darshit Chanpura <[email protected]> * Updates config schema to handle all_shared_principals to be used in search Signed-off-by: Darshit Chanpura <[email protected]> * Fix CI Signed-off-by: Darshit Chanpura <[email protected]> * Fix unit test Signed-off-by: Darshit Chanpura <[email protected]> * Updates API response message and adds method doc Signed-off-by: Darshit Chanpura <[email protected]> * Updates resource-action-groups and search handler sequence Signed-off-by: Darshit Chanpura <[email protected]> * Updates model search handler flow Signed-off-by: Darshit Chanpura <[email protected]> * Fixes method call broken during latest merge from main Signed-off-by: Darshit Chanpura <[email protected]> * Cleans up search Signed-off-by: Darshit Chanpura <[email protected]> * Reorders rsc client preference Signed-off-by: Darshit Chanpura <[email protected]> * Handles hidden models in predict action Signed-off-by: Darshit Chanpura <[email protected]> * Adds types to DocRequest Signed-off-by: Darshit Chanpura <[email protected]> * Fix prediction action error handling to pass safe modelId Signed-off-by: Darshit Chanpura <[email protected]> * Boost MLSearchHandler test coverage Signed-off-by: Darshit Chanpura <[email protected]> --------- Signed-off-by: Darshit Chanpura <[email protected]> Co-authored-by: Dhrubo Saha <[email protected]>
1 parent b9b5687 commit 7d61c3f

File tree

59 files changed

+1795
-855
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

59 files changed

+1795
-855
lines changed

‎common/build.gradle‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ dependencies {
2222
testImplementation group: 'org.mockito', name: 'mockito-core', version: '5.15.2'
2323
testImplementation "org.opensearch.test:framework:${opensearch_version}"
2424

25+
compileOnly group: 'org.opensearch', name:'opensearch-security-spi', version:"${opensearch_build}"
26+
2527
compileOnly group: 'org.apache.commons', name: 'commons-text', version: '1.10.0'
2628
compileOnly group: 'com.google.code.gson', name: 'gson', version: "${versions.gson}"
2729
compileOnly group: 'org.json', name: 'json', version: '20231013'

‎common/src/main/java/org/opensearch/ml/common/CommonValue.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,9 @@ public class CommonValue {
7979
public static final String ML_INDEX_INSIGHT_CONFIG_INDEX_MAPPING_PATH = "index-mappings/ml_index_insight_config.json";
8080
public static final String ML_INDEX_INSIGHT_STORAGE_INDEX_MAPPING_PATH = "index-mappings/ml_index_insight_storage.json";
8181

82+
// Resource type used in resource-access-control
83+
public static final String ML_MODEL_GROUP_RESOURCE_TYPE = "ml-model-group";
84+
8285
// Calculate Versions independently of OpenSearch core version
8386
public static final Version VERSION_2_11_0 = Version.fromString("2.11.0");
8487
public static final Version VERSION_2_12_0 = Version.fromString("2.12.0");

‎common/src/main/java/org/opensearch/ml/common/ResourceSharingClientAccessor.java‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package org.opensearch.ml.common;
7+
8+
import org.opensearch.security.spi.resources.client.ResourceSharingClient;
9+
10+
/**
11+
* Accessor for resource sharing client
12+
*/
13+
public class ResourceSharingClientAccessor {
14+
private ResourceSharingClient CLIENT;
15+
16+
private static ResourceSharingClientAccessor resourceSharingClientAccessor;
17+
18+
private ResourceSharingClientAccessor() {}
19+
20+
public static ResourceSharingClientAccessor getInstance() {
21+
if (resourceSharingClientAccessor == null) {
22+
resourceSharingClientAccessor = new ResourceSharingClientAccessor();
23+
}
24+
25+
return resourceSharingClientAccessor;
26+
}
27+
28+
/**
29+
* Set the resource sharing client
30+
*/
31+
public void setResourceSharingClient(ResourceSharingClient client) {
32+
resourceSharingClientAccessor.CLIENT = client;
33+
}
34+
35+
/**
36+
* Get the resource sharing client
37+
*/
38+
public ResourceSharingClient getResourceSharingClient() {
39+
return resourceSharingClientAccessor.CLIENT;
40+
}
41+
42+
}

‎common/src/main/java/org/opensearch/ml/common/transport/model_group/MLModelGroupDeleteRequest.java‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
package org.opensearch.ml.common.transport.model_group;
77

88
import static org.opensearch.action.ValidateActions.addValidationError;
9+
import static org.opensearch.ml.common.CommonValue.ML_MODEL_GROUP_INDEX;
10+
import static org.opensearch.ml.common.CommonValue.ML_MODEL_GROUP_RESOURCE_TYPE;
911
import static org.opensearch.ml.common.CommonValue.VERSION_2_19_0;
1012

1113
import java.io.ByteArrayInputStream;
@@ -16,6 +18,7 @@
1618
import org.opensearch.Version;
1719
import org.opensearch.action.ActionRequest;
1820
import org.opensearch.action.ActionRequestValidationException;
21+
import org.opensearch.action.DocRequest;
1922
import org.opensearch.core.common.io.stream.InputStreamStreamInput;
2023
import org.opensearch.core.common.io.stream.OutputStreamStreamOutput;
2124
import org.opensearch.core.common.io.stream.StreamInput;
@@ -24,7 +27,7 @@
2427
import lombok.Builder;
2528
import lombok.Getter;
2629

27-
public class MLModelGroupDeleteRequest extends ActionRequest {
30+
public class MLModelGroupDeleteRequest extends ActionRequest implements DocRequest {
2831
@Getter
2932
String modelGroupId;
3033
@Getter
@@ -78,4 +81,29 @@ public static MLModelGroupDeleteRequest fromActionRequest(ActionRequest actionRe
7881
throw new UncheckedIOException("failed to parse ActionRequest into MLModelGroupDeleteRequest", e);
7982
}
8083
}
84+
85+
@Override
86+
public String type() {
87+
return ML_MODEL_GROUP_RESOURCE_TYPE;
88+
}
89+
90+
/**
91+
* Get the index that this request operates on
92+
*
93+
* @return the index
94+
*/
95+
@Override
96+
public String index() {
97+
return ML_MODEL_GROUP_INDEX;
98+
}
99+
100+
/**
101+
* Get the id of the document for this request
102+
*
103+
* @return the id
104+
*/
105+
@Override
106+
public String id() {
107+
return modelGroupId;
108+
}
81109
}

‎common/src/main/java/org/opensearch/ml/common/transport/model_group/MLModelGroupGetRequest.java‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
package org.opensearch.ml.common.transport.model_group;
77

88
import static org.opensearch.action.ValidateActions.addValidationError;
9+
import static org.opensearch.ml.common.CommonValue.ML_MODEL_GROUP_INDEX;
10+
import static org.opensearch.ml.common.CommonValue.ML_MODEL_GROUP_RESOURCE_TYPE;
911
import static org.opensearch.ml.common.CommonValue.VERSION_2_19_0;
1012

1113
import java.io.ByteArrayInputStream;
@@ -16,6 +18,7 @@
1618
import org.opensearch.Version;
1719
import org.opensearch.action.ActionRequest;
1820
import org.opensearch.action.ActionRequestValidationException;
21+
import org.opensearch.action.DocRequest;
1922
import org.opensearch.core.common.io.stream.InputStreamStreamInput;
2023
import org.opensearch.core.common.io.stream.OutputStreamStreamOutput;
2124
import org.opensearch.core.common.io.stream.StreamInput;
@@ -30,7 +33,7 @@
3033
@Getter
3134
@FieldDefaults(makeFinal = true, level = AccessLevel.PRIVATE)
3235
@ToString
33-
public class MLModelGroupGetRequest extends ActionRequest {
36+
public class MLModelGroupGetRequest extends ActionRequest implements DocRequest {
3437

3538
String modelGroupId;
3639
String tenantId;
@@ -83,4 +86,29 @@ public static MLModelGroupGetRequest fromActionRequest(ActionRequest actionReque
8386
throw new UncheckedIOException("failed to parse ActionRequest into MLModelGroupGetRequest", e);
8487
}
8588
}
89+
90+
@Override
91+
public String type() {
92+
return ML_MODEL_GROUP_RESOURCE_TYPE;
93+
}
94+
95+
/**
96+
* Get the index that this request operates on
97+
*
98+
* @return the index
99+
*/
100+
@Override
101+
public String index() {
102+
return ML_MODEL_GROUP_INDEX;
103+
}
104+
105+
/**
106+
* Get the id of the document for this request
107+
*
108+
* @return the id
109+
*/
110+
@Override
111+
public String id() {
112+
return modelGroupId;
113+
}
86114
}

‎common/src/main/java/org/opensearch/ml/common/transport/model_group/MLUpdateModelGroupRequest.java‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
package org.opensearch.ml.common.transport.model_group;
77

88
import static org.opensearch.action.ValidateActions.addValidationError;
9+
import static org.opensearch.ml.common.CommonValue.ML_MODEL_GROUP_INDEX;
10+
import static org.opensearch.ml.common.CommonValue.ML_MODEL_GROUP_RESOURCE_TYPE;
911
import static org.opensearch.ml.common.utils.StringUtils.validateFields;
1012

1113
import java.io.ByteArrayInputStream;
@@ -17,6 +19,7 @@
1719

1820
import org.opensearch.action.ActionRequest;
1921
import org.opensearch.action.ActionRequestValidationException;
22+
import org.opensearch.action.DocRequest;
2023
import org.opensearch.core.common.io.stream.InputStreamStreamInput;
2124
import org.opensearch.core.common.io.stream.OutputStreamStreamOutput;
2225
import org.opensearch.core.common.io.stream.StreamInput;
@@ -32,7 +35,7 @@
3235
@Getter
3336
@FieldDefaults(makeFinal = true, level = AccessLevel.PRIVATE)
3437
@ToString
35-
public class MLUpdateModelGroupRequest extends ActionRequest {
38+
public class MLUpdateModelGroupRequest extends ActionRequest implements DocRequest {
3639

3740
MLUpdateModelGroupInput updateModelGroupInput;
3841

@@ -80,4 +83,29 @@ public static MLUpdateModelGroupRequest fromActionRequest(ActionRequest actionRe
8083
}
8184

8285
}
86+
87+
@Override
88+
public String type() {
89+
return ML_MODEL_GROUP_RESOURCE_TYPE;
90+
}
91+
92+
/**
93+
* Get the index that this request operates on
94+
*
95+
* @return the index
96+
*/
97+
@Override
98+
public String index() {
99+
return ML_MODEL_GROUP_INDEX;
100+
}
101+
102+
/**
103+
* Get the id of the document for this request
104+
*
105+
* @return the id
106+
*/
107+
@Override
108+
public String id() {
109+
return updateModelGroupInput.getModelGroupID();
110+
}
83111
}

‎common/src/main/resources/index-mappings/ml_model_group.json‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"_meta": {
3-
"schema_version": 3
3+
"schema_version": 4
44
},
55
"properties": {
66
"name": {
@@ -81,6 +81,9 @@
8181
"last_updated_time": {
8282
"type": "date",
8383
"format": "strict_date_time||epoch_millis"
84+
},
85+
"all_shared_principals": {
86+
"type": "keyword"
8487
}
8588
}
8689
}

‎common/src/test/java/org/opensearch/ml/common/MLModelGroupTest.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public void toXContent() throws IOException {
7373
public void parse() throws IOException {
7474
String jsonStr = "{\"name\":\"test\",\"latest_version\":1,\"description\":\"this is test group\","
7575
+ "\"backend_roles\":[\"role1\",\"role2\"],"
76-
+ "\"owner\":{\"name\":\"\",\"backend_roles\":[],\"roles\":[],\"custom_attribute_names\":[],\"user_requested_tenant\":null},"
76+
+ "\"owner\":{\"name\":\"\",\"backend_roles\":[],\"roles\":[],\"user_requested_tenant\":null,\"custom_attribute_names\":[]},"
7777
+ "\"access\":\"PUBLIC\"}";
7878
XContentParser parser = XContentType.JSON
7979
.xContent()
@@ -176,7 +176,7 @@ public void toXContent_WithTenantId() throws IOException {
176176
public void parse_WithTenantId() throws IOException {
177177
String jsonStr = "{\"name\":\"test\",\"latest_version\":1,\"description\":\"this is test group\","
178178
+ "\"backend_roles\":[\"role1\",\"role2\"],"
179-
+ "\"owner\":{\"name\":\"\",\"backend_roles\":[],\"roles\":[],\"custom_attribute_names\":[],\"user_requested_tenant\":null,\"user_requested_tenant_access\":null},"
179+
+ "\"owner\":{\"name\":\"\",\"backend_roles\":[],\"roles\":[],\"user_requested_tenant\":null,\"user_requested_tenant_access\":null,\"custom_attribute_names\":[]},"
180180
+ "\"access\":\"PUBLIC\",\"tenant_id\":\"test_tenant\"}";
181181

182182
XContentParser parser = XContentType.JSON
@@ -201,7 +201,7 @@ public void parse_WithTenantId() throws IOException {
201201
public void parse_WithoutTenantId() throws IOException {
202202
String jsonStr = "{\"name\":\"test\",\"latest_version\":1,\"description\":\"this is test group\","
203203
+ "\"backend_roles\":[\"role1\",\"role2\"],"
204-
+ "\"owner\":{\"name\":\"\",\"backend_roles\":[],\"roles\":[],\"custom_attribute_names\":[],\"user_requested_tenant\":null},"
204+
+ "\"owner\":{\"name\":\"\",\"backend_roles\":[],\"roles\":[],\"user_requested_tenant\":null,\"custom_attribute_names\":[]},"
205205
+ "\"access\":\"PUBLIC\"}";
206206

207207
XContentParser parser = XContentType.JSON

0 commit comments

Comments
 (0)