From 0711ddf1b57ed088b90830badd02b9100bc92729 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Thu, 5 Sep 2024 18:04:11 +0000 Subject: [PATCH] passing user to validation search calls (#1296) Signed-off-by: Amit Galitzky (cherry picked from commit 90a6fc3951692d5efc0073112227ba53d41a5167) Signed-off-by: github-actions[bot] --- .../timeseries/feature/SearchFeatureDao.java | 49 ++++++++++++++----- .../AbstractTimeSeriesActionHandler.java | 16 ++++-- .../rest/handler/IntervalCalculation.java | 1 - .../rest/handler/LatestTimeRetriever.java | 2 +- .../handler/ModelValidationActionHandler.java | 2 +- .../util/CrossClusterConfigUtils.java | 2 - .../ad/rest/AnomalyDetectorRestApiIT.java | 2 +- .../forecast/rest/ForecastRestApiIT.java | 2 +- 8 files changed, 53 insertions(+), 23 deletions(-) diff --git a/src/main/java/org/opensearch/timeseries/feature/SearchFeatureDao.java b/src/main/java/org/opensearch/timeseries/feature/SearchFeatureDao.java index 8c865a337..beeba3dc6 100644 --- a/src/main/java/org/opensearch/timeseries/feature/SearchFeatureDao.java +++ b/src/main/java/org/opensearch/timeseries/feature/SearchFeatureDao.java @@ -40,6 +40,7 @@ import org.opensearch.client.Client; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.settings.Settings; +import org.opensearch.commons.authuser.User; import org.opensearch.core.action.ActionListener; import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.index.query.BoolQueryBuilder; @@ -155,14 +156,28 @@ public SearchFeatureDao( * @param listener onResponse is called with the epoch time of the latest data under the detector */ public void getLatestDataTime(Config config, Optional entity, AnalysisType context, ActionListener> listener) { - BoolQueryBuilder internalFilterQuery = QueryBuilders.boolQuery(); + getLatestDataTime(null, config, entity, context, listener); + } + /** + * Returns to listener the epoch time of the latest data under the detector. + * + * @param config info about the data + * @param listener onResponse is called with the epoch time of the latest data under the detector + */ + public void getLatestDataTime( + User user, + Config config, + Optional entity, + AnalysisType context, + ActionListener> listener + ) { + BoolQueryBuilder internalFilterQuery = QueryBuilders.boolQuery(); if (entity.isPresent()) { for (TermQueryBuilder term : entity.get().getTermQueryForCustomerIndex()) { internalFilterQuery.filter(term); } } - SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder() .query(internalFilterQuery) .aggregation(AggregationBuilders.max(CommonName.AGG_NAME_MAX_TIME).field(config.getTimeField())) @@ -172,15 +187,27 @@ public void getLatestDataTime(Config config, Optional entity, AnalysisTy .wrap(response -> listener.onResponse(ParseUtils.getLatestDataTime(response)), listener::onFailure); // using the original context in listener as user roles have no permissions for internal operations like fetching a // checkpoint - clientUtil - .asyncRequestWithInjectedSecurity( - searchRequest, - client::search, - config.getId(), - client, - context, - searchResponseListener - ); + if (user != null) { + clientUtil + .asyncRequestWithInjectedSecurity( + searchRequest, + client::search, + user, + client, + context, + searchResponseListener + ); + } else { + clientUtil + .asyncRequestWithInjectedSecurity( + searchRequest, + client::search, + config.getId(), + client, + context, + searchResponseListener + ); + } } /** diff --git a/src/main/java/org/opensearch/timeseries/rest/handler/AbstractTimeSeriesActionHandler.java b/src/main/java/org/opensearch/timeseries/rest/handler/AbstractTimeSeriesActionHandler.java index 44e29f6ae..9ce014274 100644 --- a/src/main/java/org/opensearch/timeseries/rest/handler/AbstractTimeSeriesActionHandler.java +++ b/src/main/java/org/opensearch/timeseries/rest/handler/AbstractTimeSeriesActionHandler.java @@ -105,7 +105,7 @@ public abstract class AbstractTimeSeriesActionHandler>> iterator = clusterIndicesMap.entrySet().iterator(); - validateCategoricalFieldRecursive(iterator, configId, indexingDryRun, listener); + validateCategoricalField(iterator, configId, indexingDryRun, listener); } - protected void validateCategoricalFieldRecursive( + protected void validateCategoricalField( Iterator>> iterator, String configId, boolean indexingDryRun, @@ -645,13 +645,19 @@ protected void validateCategoricalFieldRecursive( listener .onFailure( createValidationException( - String.format(Locale.ROOT, CATEGORY_NOT_FOUND_ERR_MSG, categoryField0), + String + .format( + Locale.ROOT, + CATEGORY_NOT_FOUND_ERR_MSG, + categoryField0, + Arrays.toString(clusterIndicesEntry.getValue().toArray(new String[0])) + ), ValidationIssueType.CATEGORY ) ); return; } - validateCategoricalFieldRecursive(iterator, configId, indexingDryRun, listener); + validateCategoricalField(iterator, configId, indexingDryRun, listener); }, error -> { String message = String.format(Locale.ROOT, CommonMessages.FAIL_TO_GET_MAPPING_MSG, config.getIndices()); diff --git a/src/main/java/org/opensearch/timeseries/rest/handler/IntervalCalculation.java b/src/main/java/org/opensearch/timeseries/rest/handler/IntervalCalculation.java index 5a174fe59..ab17f91cf 100644 --- a/src/main/java/org/opensearch/timeseries/rest/handler/IntervalCalculation.java +++ b/src/main/java/org/opensearch/timeseries/rest/handler/IntervalCalculation.java @@ -243,7 +243,6 @@ public void onFailure(Exception e) { * interval can be a practical approach in scenarios where data arrival is irregular and there's * a need to balance between capturing data features and avoiding over-sampling. * - * @param topEntity top entity to use * @param timeStampBounds Used to determine start and end date range to search for data * @param listener returns minimum interval */ diff --git a/src/main/java/org/opensearch/timeseries/rest/handler/LatestTimeRetriever.java b/src/main/java/org/opensearch/timeseries/rest/handler/LatestTimeRetriever.java index 1086d6f76..241f4b645 100644 --- a/src/main/java/org/opensearch/timeseries/rest/handler/LatestTimeRetriever.java +++ b/src/main/java/org/opensearch/timeseries/rest/handler/LatestTimeRetriever.java @@ -81,7 +81,7 @@ public LatestTimeRetriever( * @param listener to return latest time and entity attributes if the config is HC */ public void checkIfHC(ActionListener, Map>> listener) { - searchFeatureDao.getLatestDataTime(config, Optional.empty(), context, ActionListener.wrap(latestTime -> { + searchFeatureDao.getLatestDataTime(user, config, Optional.empty(), context, ActionListener.wrap(latestTime -> { if (latestTime.isEmpty()) { listener.onResponse(Pair.of(Optional.empty(), Collections.emptyMap())); } else if (config.isHighCardinality()) { diff --git a/src/main/java/org/opensearch/timeseries/rest/handler/ModelValidationActionHandler.java b/src/main/java/org/opensearch/timeseries/rest/handler/ModelValidationActionHandler.java index 48a6b9815..226c6a778 100644 --- a/src/main/java/org/opensearch/timeseries/rest/handler/ModelValidationActionHandler.java +++ b/src/main/java/org/opensearch/timeseries/rest/handler/ModelValidationActionHandler.java @@ -147,8 +147,8 @@ public void start() { latestEntityAttributes.getRight() ), exception -> { - listener.onFailure(exception); logger.error("Failed to create search request for last data point", exception); + listener.onFailure(exception); } ); latestTimeRetriever.checkIfHC(latestTimeListener); diff --git a/src/main/java/org/opensearch/timeseries/util/CrossClusterConfigUtils.java b/src/main/java/org/opensearch/timeseries/util/CrossClusterConfigUtils.java index 61c97a814..b59e015eb 100644 --- a/src/main/java/org/opensearch/timeseries/util/CrossClusterConfigUtils.java +++ b/src/main/java/org/opensearch/timeseries/util/CrossClusterConfigUtils.java @@ -67,8 +67,6 @@ public static HashMap> separateClusterIndexes(List Pair clusterAndIndex = parseClusterAndIndexName(index); String clusterName = clusterAndIndex.getKey(); String indexName = clusterAndIndex.getValue(); - logger.info("clusterName: " + clusterName); - logger.info("indexName: " + indexName); // If the index entry does not have a cluster_name, it indicates the index is on the local cluster. if (clusterName.isEmpty()) { diff --git a/src/test/java/org/opensearch/ad/rest/AnomalyDetectorRestApiIT.java b/src/test/java/org/opensearch/ad/rest/AnomalyDetectorRestApiIT.java index 3f847f72b..05b075533 100644 --- a/src/test/java/org/opensearch/ad/rest/AnomalyDetectorRestApiIT.java +++ b/src/test/java/org/opensearch/ad/rest/AnomalyDetectorRestApiIT.java @@ -1636,7 +1636,7 @@ public void testValidateAnomalyDetectorWithWrongCategoryField() throws Exception .extractValue("detector", responseMap); assertEquals( "non-existing category", - String.format(Locale.ROOT, AbstractTimeSeriesActionHandler.CATEGORY_NOT_FOUND_ERR_MSG, "host.keyword"), + String.format(Locale.ROOT, AbstractTimeSeriesActionHandler.CATEGORY_NOT_FOUND_ERR_MSG, "host.keyword", "[index-test]"), messageMap.get("category_field").get("message") ); diff --git a/src/test/java/org/opensearch/forecast/rest/ForecastRestApiIT.java b/src/test/java/org/opensearch/forecast/rest/ForecastRestApiIT.java index 5e5dee049..aad6b2039 100644 --- a/src/test/java/org/opensearch/forecast/rest/ForecastRestApiIT.java +++ b/src/test/java/org/opensearch/forecast/rest/ForecastRestApiIT.java @@ -1709,7 +1709,7 @@ public void testValidateHC() throws Exception { assertEquals("Validate forecaster model failed", RestStatus.OK, TestHelpers.restStatus(response)); responseMap = entityAsMap(response); validations = (Map) ((Map) responseMap.get("forecaster")).get("category_field"); - assertEquals("Can't find the categorical field 476465", validations.get("message")); + assertEquals("Can't find the categorical field 476465 in index [rule]", validations.get("message")); // case 3: validate data sparsity with one categorical field forecasterDef = "{\n"