Skip to content

Commit 991fbd9

Browse files
passing user to validation search calls (#1296) (#1298)
(cherry picked from commit 90a6fc3) Signed-off-by: Amit Galitzky <[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>
1 parent 0c391f4 commit 991fbd9

File tree

8 files changed

+53
-23
lines changed

8 files changed

+53
-23
lines changed

Diff for: src/main/java/org/opensearch/timeseries/feature/SearchFeatureDao.java

+38-11
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import org.opensearch.client.Client;
4141
import org.opensearch.cluster.service.ClusterService;
4242
import org.opensearch.common.settings.Settings;
43+
import org.opensearch.commons.authuser.User;
4344
import org.opensearch.core.action.ActionListener;
4445
import org.opensearch.core.xcontent.NamedXContentRegistry;
4546
import org.opensearch.index.query.BoolQueryBuilder;
@@ -155,14 +156,28 @@ public SearchFeatureDao(
155156
* @param listener onResponse is called with the epoch time of the latest data under the detector
156157
*/
157158
public void getLatestDataTime(Config config, Optional<Entity> entity, AnalysisType context, ActionListener<Optional<Long>> listener) {
158-
BoolQueryBuilder internalFilterQuery = QueryBuilders.boolQuery();
159+
getLatestDataTime(null, config, entity, context, listener);
160+
}
159161

162+
/**
163+
* Returns to listener the epoch time of the latest data under the detector.
164+
*
165+
* @param config info about the data
166+
* @param listener onResponse is called with the epoch time of the latest data under the detector
167+
*/
168+
public void getLatestDataTime(
169+
User user,
170+
Config config,
171+
Optional<Entity> entity,
172+
AnalysisType context,
173+
ActionListener<Optional<Long>> listener
174+
) {
175+
BoolQueryBuilder internalFilterQuery = QueryBuilders.boolQuery();
160176
if (entity.isPresent()) {
161177
for (TermQueryBuilder term : entity.get().getTermQueryForCustomerIndex()) {
162178
internalFilterQuery.filter(term);
163179
}
164180
}
165-
166181
SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder()
167182
.query(internalFilterQuery)
168183
.aggregation(AggregationBuilders.max(CommonName.AGG_NAME_MAX_TIME).field(config.getTimeField()))
@@ -172,15 +187,27 @@ public void getLatestDataTime(Config config, Optional<Entity> entity, AnalysisTy
172187
.wrap(response -> listener.onResponse(ParseUtils.getLatestDataTime(response)), listener::onFailure);
173188
// using the original context in listener as user roles have no permissions for internal operations like fetching a
174189
// checkpoint
175-
clientUtil
176-
.<SearchRequest, SearchResponse>asyncRequestWithInjectedSecurity(
177-
searchRequest,
178-
client::search,
179-
config.getId(),
180-
client,
181-
context,
182-
searchResponseListener
183-
);
190+
if (user != null) {
191+
clientUtil
192+
.<SearchRequest, SearchResponse>asyncRequestWithInjectedSecurity(
193+
searchRequest,
194+
client::search,
195+
user,
196+
client,
197+
context,
198+
searchResponseListener
199+
);
200+
} else {
201+
clientUtil
202+
.<SearchRequest, SearchResponse>asyncRequestWithInjectedSecurity(
203+
searchRequest,
204+
client::search,
205+
config.getId(),
206+
client,
207+
context,
208+
searchResponseListener
209+
);
210+
}
184211
}
185212

186213
/**

Diff for: src/main/java/org/opensearch/timeseries/rest/handler/AbstractTimeSeriesActionHandler.java

+11-5
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ public abstract class AbstractTimeSeriesActionHandler<T extends ActionResponse,
105105

106106
public static final String NAME_REGEX = "[a-zA-Z0-9._-]+";
107107
public static final Integer MAX_NAME_SIZE = 64;
108-
public static final String CATEGORY_NOT_FOUND_ERR_MSG = "Can't find the categorical field %s";
108+
public static final String CATEGORY_NOT_FOUND_ERR_MSG = "Can't find the categorical field %s in index %s";
109109

110110
public static String INVALID_NAME_SIZE = "Name should be shortened. The maximum limit is "
111111
+ AbstractTimeSeriesActionHandler.MAX_NAME_SIZE
@@ -562,11 +562,11 @@ protected void validateCategoricalFieldsInAllIndices(String configId, boolean in
562562

563563
Iterator<Map.Entry<String, List<String>>> iterator = clusterIndicesMap.entrySet().iterator();
564564

565-
validateCategoricalFieldRecursive(iterator, configId, indexingDryRun, listener);
565+
validateCategoricalField(iterator, configId, indexingDryRun, listener);
566566

567567
}
568568

569-
protected void validateCategoricalFieldRecursive(
569+
protected void validateCategoricalField(
570570
Iterator<Map.Entry<String, List<String>>> iterator,
571571
String configId,
572572
boolean indexingDryRun,
@@ -645,13 +645,19 @@ protected void validateCategoricalFieldRecursive(
645645
listener
646646
.onFailure(
647647
createValidationException(
648-
String.format(Locale.ROOT, CATEGORY_NOT_FOUND_ERR_MSG, categoryField0),
648+
String
649+
.format(
650+
Locale.ROOT,
651+
CATEGORY_NOT_FOUND_ERR_MSG,
652+
categoryField0,
653+
Arrays.toString(clusterIndicesEntry.getValue().toArray(new String[0]))
654+
),
649655
ValidationIssueType.CATEGORY
650656
)
651657
);
652658
return;
653659
}
654-
validateCategoricalFieldRecursive(iterator, configId, indexingDryRun, listener);
660+
validateCategoricalField(iterator, configId, indexingDryRun, listener);
655661

656662
}, error -> {
657663
String message = String.format(Locale.ROOT, CommonMessages.FAIL_TO_GET_MAPPING_MSG, config.getIndices());

Diff for: src/main/java/org/opensearch/timeseries/rest/handler/IntervalCalculation.java

-1
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,6 @@ public void onFailure(Exception e) {
243243
* interval can be a practical approach in scenarios where data arrival is irregular and there's
244244
* a need to balance between capturing data features and avoiding over-sampling.
245245
*
246-
* @param topEntity top entity to use
247246
* @param timeStampBounds Used to determine start and end date range to search for data
248247
* @param listener returns minimum interval
249248
*/

Diff for: src/main/java/org/opensearch/timeseries/rest/handler/LatestTimeRetriever.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ public LatestTimeRetriever(
8181
* @param listener to return latest time and entity attributes if the config is HC
8282
*/
8383
public void checkIfHC(ActionListener<Pair<Optional<Long>, Map<String, Object>>> listener) {
84-
searchFeatureDao.getLatestDataTime(config, Optional.empty(), context, ActionListener.wrap(latestTime -> {
84+
searchFeatureDao.getLatestDataTime(user, config, Optional.empty(), context, ActionListener.wrap(latestTime -> {
8585
if (latestTime.isEmpty()) {
8686
listener.onResponse(Pair.of(Optional.empty(), Collections.emptyMap()));
8787
} else if (config.isHighCardinality()) {

Diff for: src/main/java/org/opensearch/timeseries/rest/handler/ModelValidationActionHandler.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -147,8 +147,8 @@ public void start() {
147147
latestEntityAttributes.getRight()
148148
),
149149
exception -> {
150-
listener.onFailure(exception);
151150
logger.error("Failed to create search request for last data point", exception);
151+
listener.onFailure(exception);
152152
}
153153
);
154154
latestTimeRetriever.checkIfHC(latestTimeListener);

Diff for: src/main/java/org/opensearch/timeseries/util/CrossClusterConfigUtils.java

-2
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,6 @@ public static HashMap<String, List<String>> separateClusterIndexes(List<String>
6767
Pair<String, String> clusterAndIndex = parseClusterAndIndexName(index);
6868
String clusterName = clusterAndIndex.getKey();
6969
String indexName = clusterAndIndex.getValue();
70-
logger.info("clusterName: " + clusterName);
71-
logger.info("indexName: " + indexName);
7270

7371
// If the index entry does not have a cluster_name, it indicates the index is on the local cluster.
7472
if (clusterName.isEmpty()) {

Diff for: src/test/java/org/opensearch/ad/rest/AnomalyDetectorRestApiIT.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -1636,7 +1636,7 @@ public void testValidateAnomalyDetectorWithWrongCategoryField() throws Exception
16361636
.extractValue("detector", responseMap);
16371637
assertEquals(
16381638
"non-existing category",
1639-
String.format(Locale.ROOT, AbstractTimeSeriesActionHandler.CATEGORY_NOT_FOUND_ERR_MSG, "host.keyword"),
1639+
String.format(Locale.ROOT, AbstractTimeSeriesActionHandler.CATEGORY_NOT_FOUND_ERR_MSG, "host.keyword", "[index-test]"),
16401640
messageMap.get("category_field").get("message")
16411641
);
16421642

Diff for: src/test/java/org/opensearch/forecast/rest/ForecastRestApiIT.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -1709,7 +1709,7 @@ public void testValidateHC() throws Exception {
17091709
assertEquals("Validate forecaster model failed", RestStatus.OK, TestHelpers.restStatus(response));
17101710
responseMap = entityAsMap(response);
17111711
validations = (Map<String, Object>) ((Map<String, Object>) responseMap.get("forecaster")).get("category_field");
1712-
assertEquals("Can't find the categorical field 476465", validations.get("message"));
1712+
assertEquals("Can't find the categorical field 476465 in index [rule]", validations.get("message"));
17131713

17141714
// case 3: validate data sparsity with one categorical field
17151715
forecasterDef = "{\n"

0 commit comments

Comments
 (0)