Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding reprovision integration tests #834

Merged
merged 21 commits into from
Aug 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,13 @@
);
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.",

Check warning on line 143 in src/main/java/org/opensearch/flowframework/rest/RestCreateWorkflowAction.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/flowframework/rest/RestCreateWorkflowAction.java#L142-L143

Added lines #L142 - L143 were not covered by tests
RestStatus.BAD_REQUEST
);
return processError(ffe, params, request);

Check warning on line 146 in src/main/java/org/opensearch/flowframework/rest/RestCreateWorkflowAction.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/flowframework/rest/RestCreateWorkflowAction.java#L146

Added line #L146 was not covered by tests
}
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
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;
}
}
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) {
joshpalis marked this conversation as resolved.
Show resolved Hide resolved
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
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
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
Loading