Skip to content

Commit

Permalink
Adding reprovision integration tests (#834)
Browse files Browse the repository at this point in the history 
* Adding reprovision integration tests

Signed-off-by: Joshua Palis <jpalis@amazon.com>

* spotless

Signed-off-by: Joshua Palis <jpalis@amazon.com>

* Adding deprovision/delete to reprovision integration tests

Signed-off-by: Joshua Palis <jpalis@amazon.com>

* Adding deprovision/delete to reprovision failure tests

Signed-off-by: Joshua Palis <jpalis@amazon.com>

* Using remote models rather than local models to reduce flakiness

Signed-off-by: Joshua Palis <jpalis@amazon.com>

* Fixing forbiddenApis check

Signed-off-by: Joshua Palis <jpalis@amazon.com>

* Fixing forbiddenAPI check, addressing PR comments

Signed-off-by: Joshua Palis <jpalis@amazon.com>

* Fixing forbiddenAPIs main

Signed-off-by: Joshua Palis <jpalis@amazon.com>

* increasing getResource timeout

Signed-off-by: Joshua Palis <jpalis@amazon.com>

* Addressing PR comments

Signed-off-by: Joshua Palis <jpalis@amazon.com>

* Fixing multi-node integration tests

Signed-off-by: Joshua Palis <jpalis@amazon.com>

* fixing multi-node integration tests

Signed-off-by: Joshua Palis <jpalis@amazon.com>

* Fixing syntax error

Signed-off-by: Joshua Palis <jpalis@amazon.com>

* Blocking reprovision requests with substitution params

Signed-off-by: Joshua Palis <jpalis@amazon.com>

* Fixes update settings request issue for multi-node

Signed-off-by: Joshua Palis <jpalis@amazon.com>

* Increasing test coverage

Signed-off-by: Joshua Palis <jpalis@amazon.com>

* Adding return to javadoc

Signed-off-by: Joshua Palis <jpalis@amazon.com>

* Adding test coverage

Signed-off-by: Joshua Palis <jpalis@amazon.com>

* Increasing test coverage

Signed-off-by: Joshua Palis <jpalis@amazon.com>

* Increasing test coverage

Signed-off-by: Joshua Palis <jpalis@amazon.com>

---------

Signed-off-by: Joshua Palis <jpalis@amazon.com>
  • Loading branch information
@joshpalis
joshpalis authored Aug 16, 2024
1 parent dc20feb commit 562d476
Show file tree
Hide file tree
Showing 15 changed files with 894 additions and 89 deletions.
6 changes: 3 additions & 3 deletions src/main/java/org/opensearch/flowframework/model/Template.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@
*/
package org.opensearch.flowframework.model;

import org.apache.logging.log4j.util.Strings;
import org.opensearch.Version;
import org.opensearch.common.xcontent.LoggingDeprecationHandler;
import org.opensearch.common.xcontent.json.JsonXContent;
import org.opensearch.common.xcontent.yaml.YamlXContent;
import org.opensearch.commons.authuser.User;
import org.opensearch.core.common.Strings;
import org.opensearch.core.rest.RestStatus;
import org.opensearch.core.xcontent.NamedXContentRegistry;
import org.opensearch.core.xcontent.ToXContentObject;
Expand Down Expand Up @@ -372,10 +372,10 @@ public static Template updateExistingTemplate(Template existingTemplate, Templat
if (templateWithNewFields.name() != null) {
builder.name(templateWithNewFields.name());
}
if (!Strings.isBlank(templateWithNewFields.description())) {
if (Strings.hasText(templateWithNewFields.description())) {
builder.description(templateWithNewFields.description());
}
if (!Strings.isBlank(templateWithNewFields.useCase())) {
if (Strings.hasText(templateWithNewFields.useCase())) {
builder.useCase(templateWithNewFields.useCase());
}
if (templateWithNewFields.templateVersion() != null) {
Expand Down
7 changes: 7 additions & 0 deletions src/main/java/org/opensearch/flowframework/rest/RestCreateWorkflowAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,13 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli
);
return processError(ffe, params, request);
}
if (reprovision && !params.isEmpty()) {
FlowFrameworkException ffe = new FlowFrameworkException(
"Only the parameters " + request.consumedParams() + " are permitted unless the provision parameter is set to true.",
RestStatus.BAD_REQUEST
);
return processError(ffe, params, request);
}
try {
Template template;
Map<String, String> useCaseDefaultsMap = Collections.emptyMap();
Expand Down
17 changes: 17 additions & 0 deletions src/main/java/org/opensearch/flowframework/util/ParseUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -533,4 +533,21 @@ public static void flattenSettings(String prefix, Map<String, Object> settings,
}
}
}

/**
* Ensures index is prepended to flattened setting keys
* @param originalSettings the original settings map
* @return new map with keys prepended with index
*/
public static Map<String, Object> prependIndexToSettings(Map<String, Object> originalSettings) {
Map<String, Object> newSettings = new HashMap<>();
originalSettings.entrySet().stream().forEach(x -> {
if (!x.getKey().startsWith("index.")) {
newSettings.put("index." + x.getKey(), x.getValue());
} else {
newSettings.put(x.getKey(), x.getValue());
}
});
return newSettings;
}
}
51 changes: 29 additions & 22 deletions src/main/java/org/opensearch/flowframework/workflow/UpdateIndexStep.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,10 @@ public PlainActionFuture<WorkflowData> execute(
if (updatedSettings.containsKey("index")) {
ParseUtils.flattenSettings("", updatedSettings, flattenedSettings);
} else {
flattenedSettings.putAll(updatedSettings);
// Create index setting configuration can be a mix of flattened or expanded settings
// prepend index. to ensure successful setting comparison

flattenedSettings.putAll(ParseUtils.prependIndexToSettings(updatedSettings));
}

Map<String, Object> filteredSettings = new HashMap<>();
Expand All @@ -133,35 +136,39 @@ public PlainActionFuture<WorkflowData> execute(
filteredSettings.put(e.getKey(), e.getValue());
}
}

// Create and send the update settings request
updateSettingsRequest.settings(filteredSettings);
if (updateSettingsRequest.settings().size() == 0) {
String errorMessage = "Failed to update index settings for index "
+ indexName
+ ", no settings have been updated";
updateIndexFuture.onFailure(new WorkflowStepException(errorMessage, RestStatus.BAD_REQUEST));
} else {
client.admin().indices().updateSettings(updateSettingsRequest, ActionListener.wrap(acknowledgedResponse -> {
String resourceName = getResourceByWorkflowStep(getName());
logger.info("Updated index settings for index {}", indexName);
updateIndexFuture.onResponse(
new WorkflowData(Map.of(resourceName, indexName), currentNodeInputs.getWorkflowId(), currentNodeId)
);

}, ex -> {
Exception e = getSafeException(ex);
String errorMessage = (e == null
? "Failed to update the index settings for index " + indexName
: e.getMessage());
logger.error(errorMessage, e);
updateIndexFuture.onFailure(new WorkflowStepException(errorMessage, ExceptionsHelper.status(e)));
}));
}
}, ex -> {
Exception e = getSafeException(ex);
String errorMessage = (e == null ? "Failed to retrieve the index settings for index " + indexName : e.getMessage());
logger.error(errorMessage, e);
updateIndexFuture.onFailure(new WorkflowStepException(errorMessage, ExceptionsHelper.status(e)));
}));

updateSettingsRequest.settings(filteredSettings);
}
}

if (updateSettingsRequest.settings().size() == 0) {
String errorMessage = "Failed to update index settings for index " + indexName + ", no settings have been updated";
throw new FlowFrameworkException(errorMessage, RestStatus.BAD_REQUEST);
} else {
client.admin().indices().updateSettings(updateSettingsRequest, ActionListener.wrap(acknowledgedResponse -> {
String resourceName = getResourceByWorkflowStep(getName());
logger.info("Updated index settings for index {}", indexName);
updateIndexFuture.onResponse(
new WorkflowData(Map.of(resourceName, indexName), currentNodeInputs.getWorkflowId(), currentNodeId)
);

}, ex -> {
Exception e = getSafeException(ex);
String errorMessage = (e == null ? "Failed to update the index settings for index " + indexName : e.getMessage());
logger.error(errorMessage, e);
updateIndexFuture.onFailure(new WorkflowStepException(errorMessage, ExceptionsHelper.status(e)));
}));
}
} catch (Exception e) {
updateIndexFuture.onFailure(new WorkflowStepException(e.getMessage(), ExceptionsHelper.status(e)));
}
Expand Down
19 changes: 19 additions & 0 deletions src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,25 @@ protected Response createWorkflowValidation(RestClient client, Template template
return TestHelpers.makeRequest(client, "POST", WORKFLOW_URI, Collections.emptyMap(), template.toJson(), null);
}

/**
* Helper method to invoke the Reprovision Workflow API
* @param client the rest client
* @param workflowId the document id
* @param templateFields the template to reprovision
* @throws Exception if the request fails
* @return a rest response
*/
protected Response reprovisionWorkflow(RestClient client, String workflowId, Template template) throws Exception {
return TestHelpers.makeRequest(
client,
"PUT",
String.format(Locale.ROOT, "%s/%s?reprovision=true", WORKFLOW_URI, workflowId),
Collections.emptyMap(),
template.toJson(),
null
);
}

/**
* Helper method to invoke the Update Workflow API
* @param client the rest client
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/org/opensearch/flowframework/TestHelpers.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import org.apache.hc.core5.http.Header;
import org.apache.hc.core5.http.HttpEntity;
import org.apache.hc.core5.http.io.entity.StringEntity;
import org.apache.logging.log4j.util.Strings;
import org.opensearch.client.Request;
import org.opensearch.client.RequestOptions;
import org.opensearch.client.Response;
Expand All @@ -24,6 +23,7 @@
import org.opensearch.common.xcontent.XContentHelper;
import org.opensearch.common.xcontent.XContentType;
import org.opensearch.commons.authuser.User;
import org.opensearch.core.common.Strings;
import org.opensearch.core.common.bytes.BytesReference;
import org.opensearch.core.rest.RestStatus;
import org.opensearch.core.xcontent.ToXContent;
Expand Down Expand Up @@ -74,7 +74,7 @@ public static Response makeRequest(
String jsonEntity,
List<Header> headers
) throws IOException {
HttpEntity httpEntity = Strings.isBlank(jsonEntity) ? null : new StringEntity(jsonEntity, APPLICATION_JSON);
HttpEntity httpEntity = !Strings.hasText(jsonEntity) ? null : new StringEntity(jsonEntity, APPLICATION_JSON);
return makeRequest(client, method, endpoint, params, httpEntity, headers);
}

Expand Down
Loading

0 comments on commit 562d476

Please sign in to comment.