Skip to content

Commit 914a58f

Browse files
Introduces resource sharing model as a feature flag
Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
1 parent f965695 commit 914a58f

32 files changed

+442
-118
lines changed

CHANGELOG.md

-6
Original file line numberDiff line numberDiff line change
@@ -8,22 +8,16 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.1.0/)
88
### Enhancements
99
### Bug Fixes
1010
### Infrastructure
11-
1211
### Documentation
13-
1412
### Maintenance
15-
- Fix breaking changes for 3.0.0 release ([#1424](https://github.com/opensearch-project/anomaly-detection/pull/1424))
16-
1713
### Refactoring
1814

1915
## [Unreleased 2.x](https://github.com/opensearch-project/anomaly-detection/compare/2.19...2.x)
2016
### Features
2117

22-
2318
### Enhancements
2419
- Github workflow for changelog verification ([#1413](https://github.com/opensearch-project/anomaly-detection/pull/1413))
2520
### Bug Fixes
26-
2721
### Infrastructure
2822
### Documentation
2923
### Maintenance

build.gradle

+2-2
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ buildscript {
6565
}
6666

6767
plugins {
68-
id 'com.netflix.nebula.ospackage' version "11.10.0"
68+
id 'com.netflix.nebula.ospackage' version "11.11.1"
6969
id "com.diffplug.spotless" version "6.25.0"
7070
id 'java-library'
7171
id 'org.gradle.test-retry' version '1.6.0'
@@ -115,7 +115,7 @@ dependencies {
115115
implementation "org.opensearch:opensearch:${opensearch_version}"
116116
compileOnly "org.opensearch.plugin:opensearch-scripting-painless-spi:${opensearch_version}"
117117
compileOnly "org.opensearch:opensearch-job-scheduler-spi:${job_scheduler_version}"
118-
compileOnly "org.opensearch:opensearch-resource-sharing-spi:${opensearch_build}"
118+
compileOnly "org.opensearch:opensearch-security-client:${opensearch_build}"
119119
implementation "org.opensearch:common-utils:${common_utils_version}"
120120
implementation "org.opensearch.client:opensearch-rest-client:${opensearch_version}"
121121
compileOnly group: 'com.google.guava', name: 'guava', version:'32.1.3-jre'
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
## Version 3.0.0.0-alpha1 Release Notes
2+
3+
Compatible with OpenSearch 3.0.0.0-alpha1
4+
5+
### Maintenance
6+
- Fix breaking changes for 3.0.0 release ([#1424](https://github.com/opensearch-project/anomaly-detection/pull/1424))

src/main/java/org/opensearch/ad/constant/ADResourceScope.java

+11
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,14 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*
8+
* Modifications Copyright OpenSearch Contributors. See
9+
* GitHub history for details.
10+
*/
11+
112
package org.opensearch.ad.constant;
213

314
import org.opensearch.security.spi.resources.ResourceAccessScope;

src/main/java/org/opensearch/ad/constant/ConfigConstants.java

+11
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,14 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*
8+
* Modifications Copyright OpenSearch Contributors. See
9+
* GitHub history for details.
10+
*/
11+
112
package org.opensearch.ad.constant;
213

314
public class ConfigConstants {

src/main/java/org/opensearch/ad/model/AnomalyDetector.java

+18-1
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import org.opensearch.core.xcontent.XContentParser;
4343
import org.opensearch.index.query.QueryBuilder;
4444
import org.opensearch.index.query.QueryBuilders;
45+
import org.opensearch.security.spi.resources.Resource;
4546
import org.opensearch.timeseries.annotation.Generated;
4647
import org.opensearch.timeseries.common.exception.ValidationException;
4748
import org.opensearch.timeseries.constant.CommonMessages;
@@ -66,7 +67,8 @@
6667
* TODO: Will replace detector config mapping in AD task with detector config setting directly \
6768
* in code rather than config it in anomaly-detection-state.json file.
6869
*/
69-
public class AnomalyDetector extends Config {
70+
public class AnomalyDetector extends Config implements Resource {
71+
7072
static class ADShingleGetter implements ShingleGetter {
7173
private Integer seasonIntervals;
7274

@@ -240,6 +242,21 @@ public AnomalyDetector(
240242
this.rules = rules == null || rules.isEmpty() ? getDefaultRule() : rules;
241243
}
242244

245+
@Override
246+
public String getResourceName() {
247+
return "detector";
248+
}
249+
250+
@Override
251+
public String getWriteableName() {
252+
return "anomaly_detector";
253+
}
254+
255+
@Override
256+
public boolean isFragment() {
257+
return super.isFragment();
258+
}
259+
243260
/*
244261
* For backward compatiblity reason, we cannot use super class
245262
* Config's constructor as we have detectionDateRange and

src/main/java/org/opensearch/ad/transport/AnomalyDetectorJobTransportAction.java

+5-2
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.opensearch.timeseries.transport.BaseJobTransportAction;
3535
import org.opensearch.transport.TransportService;
3636
import org.opensearch.transport.client.Client;
37+
import org.opensearch.transport.client.node.NodeClient;
3738

3839
public class AnomalyDetectorJobTransportAction extends
3940
BaseJobTransportAction<ADIndex, ADIndexManagement, ADTaskCacheManager, ADTaskType, ADTask, ADTaskManager, AnomalyResult, ADProfileAction, ExecuteADResultResponseRecorder, ADIndexJobActionHandler> {
@@ -45,7 +46,8 @@ public AnomalyDetectorJobTransportAction(
4546
ClusterService clusterService,
4647
Settings settings,
4748
NamedXContentRegistry xContentRegistry,
48-
ADIndexJobActionHandler adIndexJobActionHandler
49+
ADIndexJobActionHandler adIndexJobActionHandler,
50+
NodeClient nodeClient
4951
) {
5052
super(
5153
transportService,
@@ -60,7 +62,8 @@ public AnomalyDetectorJobTransportAction(
6062
FAIL_TO_START_DETECTOR,
6163
FAIL_TO_STOP_DETECTOR,
6264
AnomalyDetector.class,
63-
adIndexJobActionHandler
65+
adIndexJobActionHandler,
66+
nodeClient
6467
);
6568
}
6669
}

src/main/java/org/opensearch/ad/transport/DeleteAnomalyDetectorTransportAction.java

+5-2
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import org.opensearch.timeseries.transport.BaseDeleteConfigTransportAction;
3131
import org.opensearch.transport.TransportService;
3232
import org.opensearch.transport.client.Client;
33+
import org.opensearch.transport.client.node.NodeClient;
3334

3435
public class DeleteAnomalyDetectorTransportAction extends
3536
BaseDeleteConfigTransportAction<ADTaskCacheManager, ADTaskType, ADTask, ADIndex, ADIndexManagement, ADTaskManager, AnomalyDetector> {
@@ -43,7 +44,8 @@ public DeleteAnomalyDetectorTransportAction(
4344
Settings settings,
4445
NamedXContentRegistry xContentRegistry,
4546
NodeStateManager nodeStateManager,
46-
ADTaskManager adTaskManager
47+
ADTaskManager adTaskManager,
48+
NodeClient nodeClient
4749
) {
4850
super(
4951
transportService,
@@ -59,7 +61,8 @@ public DeleteAnomalyDetectorTransportAction(
5961
AnalysisType.AD,
6062
ADCommonName.DETECTION_STATE_INDEX,
6163
AnomalyDetector.class,
62-
ADTaskType.HISTORICAL_DETECTOR_TASK_TYPES
64+
ADTaskType.HISTORICAL_DETECTOR_TASK_TYPES,
65+
nodeClient
6366
);
6467
}
6568
}

src/main/java/org/opensearch/ad/transport/GetAnomalyDetectorTransportAction.java

+5-2
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
import org.opensearch.timeseries.util.SecurityClientUtil;
4444
import org.opensearch.transport.TransportService;
4545
import org.opensearch.transport.client.Client;
46+
import org.opensearch.transport.client.node.NodeClient;
4647

4748
public class GetAnomalyDetectorTransportAction extends
4849
BaseGetConfigTransportAction<GetAnomalyDetectorResponse, ADTaskCacheManager, ADTaskType, ADTask, ADIndex, ADIndexManagement, ADTaskManager, AnomalyDetector, ADEntityProfileAction, ADEntityProfileRunner, ADTaskProfile, DetectorProfile, ADProfileAction, ADTaskProfileRunner, AnomalyDetectorProfileRunner> {
@@ -60,7 +61,8 @@ public GetAnomalyDetectorTransportAction(
6061
Settings settings,
6162
NamedXContentRegistry xContentRegistry,
6263
ADTaskManager adTaskManager,
63-
ADTaskProfileRunner adTaskProfileRunner
64+
ADTaskProfileRunner adTaskProfileRunner,
65+
NodeClient nodeClient
6466
) {
6567
super(
6668
transportService,
@@ -81,7 +83,8 @@ public GetAnomalyDetectorTransportAction(
8183
ADTaskType.HISTORICAL_HC_DETECTOR.name(),
8284
ADTaskType.HISTORICAL_SINGLE_ENTITY.name(),
8385
AnomalyDetectorSettings.AD_FILTER_BY_BACKEND_ROLES,
84-
adTaskProfileRunner
86+
adTaskProfileRunner,
87+
nodeClient
8588
);
8689
}
8790

src/main/java/org/opensearch/ad/transport/IndexAnomalyDetectorTransportAction.java

+63-28
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import static org.opensearch.ad.settings.AnomalyDetectorSettings.AD_FILTER_BY_BACKEND_ROLES;
1717
import static org.opensearch.timeseries.util.ParseUtils.checkFilterByBackendRoles;
1818
import static org.opensearch.timeseries.util.ParseUtils.getConfig;
19+
import static org.opensearch.timeseries.util.ParseUtils.verifyResourceAccessAndProcessRequest;
1920
import static org.opensearch.timeseries.util.RestHandlerUtils.wrapRestActionListener;
2021

2122
import java.util.List;
@@ -27,6 +28,7 @@
2728
import org.opensearch.action.support.ActionFilters;
2829
import org.opensearch.action.support.HandledTransportAction;
2930
import org.opensearch.action.support.WriteRequest;
31+
import org.opensearch.ad.constant.ADResourceScope;
3032
import org.opensearch.ad.constant.ConfigConstants;
3133
import org.opensearch.ad.indices.ADIndexManagement;
3234
import org.opensearch.ad.model.AnomalyDetector;
@@ -52,6 +54,7 @@
5254
import org.opensearch.timeseries.util.SecurityClientUtil;
5355
import org.opensearch.transport.TransportService;
5456
import org.opensearch.transport.client.Client;
57+
import org.opensearch.transport.client.node.NodeClient;
5558

5659
public class IndexAnomalyDetectorTransportAction extends HandledTransportAction<IndexAnomalyDetectorRequest, IndexAnomalyDetectorResponse> {
5760
private static final Logger LOG = LogManager.getLogger(IndexAnomalyDetectorTransportAction.class);
@@ -66,6 +69,7 @@ public class IndexAnomalyDetectorTransportAction extends HandledTransportAction<
6669
private final SearchFeatureDao searchFeatureDao;
6770
private final Settings settings;
6871
private final boolean resourceSharingEnabled;
72+
private final NodeClient nodeClient;
6973

7074
@Inject
7175
public IndexAnomalyDetectorTransportAction(
@@ -78,7 +82,8 @@ public IndexAnomalyDetectorTransportAction(
7882
ADIndexManagement anomalyDetectionIndices,
7983
NamedXContentRegistry xContentRegistry,
8084
ADTaskManager adTaskManager,
81-
SearchFeatureDao searchFeatureDao
85+
SearchFeatureDao searchFeatureDao,
86+
NodeClient nodeClient
8287
) {
8388
super(IndexAnomalyDetectorAction.NAME, transportService, actionFilters, IndexAnomalyDetectorRequest::new);
8489
this.client = client;
@@ -94,6 +99,7 @@ public IndexAnomalyDetectorTransportAction(
9499
this.settings = settings;
95100
this.resourceSharingEnabled = settings
96101
.getAsBoolean(ConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED, ConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED_DEFAULT);
102+
this.nodeClient = nodeClient;
97103
}
98104

99105
@Override
@@ -103,7 +109,29 @@ protected void doExecute(Task task, IndexAnomalyDetectorRequest request, ActionL
103109
RestRequest.Method method = request.getMethod();
104110
String errorMessage = method == RestRequest.Method.PUT ? FAIL_TO_UPDATE_DETECTOR : FAIL_TO_CREATE_DETECTOR;
105111
ActionListener<IndexAnomalyDetectorResponse> listener = wrapRestActionListener(actionListener, errorMessage);
112+
106113
try (ThreadContext.StoredContext context = client.threadPool().getThreadContext().stashContext()) {
114+
if (resourceSharingEnabled) {
115+
// Call verifyResourceAccessAndProcessRequest before proceeding
116+
verifyResourceAccessAndProcessRequest(
117+
user,
118+
detectorId,
119+
ADResourceScope.AD_FULL_ACCESS.value(),
120+
nodeClient,
121+
settings,
122+
listener,
123+
args -> indexDetector(
124+
user,
125+
detectorId,
126+
method,
127+
listener,
128+
detector -> adExecute(request, user, detector, context, listener)
129+
) // Execute only if access is granted
130+
);
131+
return;
132+
}
133+
134+
// Proceed with normal execution if resource sharing is not enabled
107135
resolveUserAndExecute(user, detectorId, method, listener, (detector) -> adExecute(request, user, detector, context, listener));
108136
} catch (Exception e) {
109137
LOG.error(e);
@@ -119,44 +147,51 @@ private void resolveUserAndExecute(
119147
Consumer<AnomalyDetector> function
120148
) {
121149
try {
122-
// If resource sharing flag is enabled then access evaluation will be performed at DLS level
123-
if (!resourceSharingEnabled && filterByEnabled) {
124-
// Check if user has backend roles
125-
// When filter by is enabled, block users creating/updating detectors who do not have backend roles.
150+
if (filterByEnabled) {
126151
String error = checkFilterByBackendRoles(requestedUser);
127152
if (error != null) {
128153
listener.onFailure(new TimeSeriesException(error));
129154
return;
130155
}
131156
}
132-
if (method == RestRequest.Method.PUT) {
133-
// requestedUser == null means security is disabled or user is superadmin. In this case we don't need to
134-
// check if request user have access to the detector or not. But we still need to get current detector for
135-
// this case, so we can keep current detector's user data.
136-
boolean filterByBackendRole = requestedUser == null ? false : filterByEnabled;
137-
// Update detector request, check if user has permissions to update the detector
138-
// Get detector and verify backend roles
139-
getConfig(
140-
requestedUser,
141-
detectorId,
142-
listener,
143-
function,
144-
client,
145-
clusterService,
146-
xContentRegistry,
147-
filterByBackendRole,
148-
AnomalyDetector.class,
149-
resourceSharingEnabled
150-
);
151-
} else {
152-
// Create Detector. No need to get current detector.
153-
function.accept(null);
154-
}
157+
158+
indexDetector(requestedUser, detectorId, method, listener, function);
155159
} catch (Exception e) {
156160
listener.onFailure(e);
157161
}
158162
}
159163

164+
private void indexDetector(
165+
User requestedUser,
166+
String detectorId,
167+
RestRequest.Method method,
168+
ActionListener<IndexAnomalyDetectorResponse> listener,
169+
Consumer<AnomalyDetector> function
170+
) {
171+
if (method == RestRequest.Method.PUT) {
172+
// requestedUser == null means security is disabled or user is superadmin. In this case we don't need to
173+
// check if request user have access to the detector or not. But we still need to get current detector for
174+
// this case, so we can keep current detector's user data.
175+
boolean filterByBackendRole = requestedUser == null ? false : filterByEnabled;
176+
// Update detector request, check if user has permissions to update the detector
177+
// Get detector and verify backend roles
178+
getConfig(
179+
requestedUser,
180+
detectorId,
181+
listener,
182+
function,
183+
client,
184+
clusterService,
185+
xContentRegistry,
186+
filterByBackendRole,
187+
AnomalyDetector.class
188+
);
189+
} else {
190+
// Create Detector. No need to get current detector.
191+
function.accept(null);
192+
}
193+
}
194+
160195
protected void adExecute(
161196
IndexAnomalyDetectorRequest request,
162197
User user,

0 commit comments

Comments
 (0)